lgtm
a few comments below; feel free to ignore them.
https://codereview.chromium.org/874393007/diff/60001/BUILD.gn
File BUILD.gn (right):
https://codereview.chromium.org/874393007/diff/60001/BUILD.gn#newcode37
BUILD.gn:37: assert(host_os == "linux")
This is definitely simpler and an improvement.
https://codereview.chromium.org/874393007/diff/60001/BUILD.gn#newcode128
BUILD.gn:128: if (arm_fpu == "vfpv3") {
nit: I'd probably write lines 128 and 134 as 'else if' to make it a
little more obvious that these choices are mutually exclusive (though
it's obviously not actually necessary), and put a blank line between
lines 124 and 125 to make it clearer that these are two different sets
of conditionals.
https://codereview.chromium.org/874393007/diff/60001/BUILD.gn#newcode142
BUILD.gn:142: defines += [
We only get into this branch when building mksnapshot (and its deps) in
the host toolchain, right?
It might be helpful to put a comment to that effect in here, something
like:
# these defines are used in the ARM snapshot_toolchain.
https://codereview.chromium.org/874393007/
--
--
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.
