https://codereview.chromium.org/293993021/diff/1/include/v8.h
File include/v8.h (right):
https://codereview.chromium.org/293993021/diff/1/include/v8.h#newcode4668
include/v8.h:4668: #ifdef V8_USE_EXTERNAL_STARTUP_DATA
embedders won't see this #define, the api should always be present
https://codereview.chromium.org/293993021/diff/1/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/293993021/diff/1/src/api.cc#newcode350
src/api.cc:350: #ifdef V8_USE_EXTERNAL_STARTUP_DATA
only #ifdef the function body (and maybe add an CHECK(false) in the
#else part
https://codereview.chromium.org/293993021/diff/1/src/d8.cc
File src/d8.cc (right):
https://codereview.chromium.org/293993021/diff/1/src/d8.cc#newcode1320
src/d8.cc:1320: else if (strncmp(argv[i], "--natives_blob=", 15) == 0) {
using - is more consistent with the other options (as opposed to _)
https://codereview.chromium.org/293993021/diff/1/src/d8.cc#newcode1528
src/d8.cc:1528: };
nit. DISALLOW_COPY_AND_ASSIGN
https://codereview.chromium.org/293993021/diff/1/src/mksnapshot.cc
File src/mksnapshot.cc (right):
https://codereview.chromium.org/293993021/diff/1/src/mksnapshot.cc#newcode316
src/mksnapshot.cc:316: void SetSnapshotFromFile(StartupData* data) {
ASSERT(false); }
i guess this should go into snapshot-empty.cc?
https://codereview.chromium.org/293993021/diff/1/src/snapshot-source-sink.h
File src/snapshot-source-sink.h (right):
https://codereview.chromium.org/293993021/diff/1/src/snapshot-source-sink.h#newcode22
src/snapshot-source-sink.h:22: SnapshotByteSource(const byte* array, int
length);
please add a dtor
https://codereview.chromium.org/293993021/diff/1/src/snapshot-source-sink.h#newcode60
src/snapshot-source-sink.h:60: };
disallow copy and assign
https://codereview.chromium.org/293993021/diff/1/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):
https://codereview.chromium.org/293993021/diff/1/tools/gyp/v8.gyp#newcode624
tools/gyp/v8.gyp:624: '../../src/snapshot-source-sink.cc',
can you do those changes (adding/removing files) in BUILD.gn as well? It
doesn't have the snapshot generation logic or anything beyond that yet,
so just adding the files to the list will be enough.
https://codereview.chromium.org/293993021/diff/1/tools/gyp/v8.gyp#newcode1136
tools/gyp/v8.gyp:1136: '<(SHARED_INTERMEDIATE_DIR)/libraries.bin',
I think we want those in <(PRODUCT_DIR)
https://codereview.chromium.org/293993021/diff/1/tools/gyp/v8.gyp#newcode1143
tools/gyp/v8.gyp:1143: 'bash',
there's no bash on windows. gyp has some magic to replace cat with type
on windows, however, i'm not sure whether that can cope with binary
files, so you may want to write a small python script for that.
https://codereview.chromium.org/293993021/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
