On 2014/11/24 21:35:13, baixo1 wrote:
Addressed Ross' comments.

https://codereview.chromium.org/749213004/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/749213004/diff/1/BUILD.gn#newcode1418
BUILD.gn:1418: deps = []
On 2014/11/24 18:01:34, rmcilroy wrote:
> On 2014/11/24 17:28:11, baixo1 wrote:
> > On 2014/11/24 17:21:06, rmcilroy wrote:
> > > Is there any reason you need to make these changes?  I would have
thought
> the
> > > if/else if/else structure before would have given you the same result?
> >
> > I thought the same :). But no :)
> > When you have the structure:
> >
> > if (v8_use_external_startup_data)
> >   do something;
> > else if (v8_snapshot)
> >   do something else;
> > else
> >   do another thing;
> >
> > if v8_use_external_startup_data is true, it won't consider the other
branches
> > exist. And then it'll complain that v8_snapshot was defined but never used
:)
>
> Eww, yuck... Ohh well, perhaps you could just make it
>   if (v8_use_snapshot && v8_use_external_startup_data) {
> Since I think v8_use_snapshot has to be true for
v8_use_external_startup_data
to
> be used (and you could assert !v8_use_external_startup_data in the final
"else"
> branch.

Done.

lgtm, but please change the title description to "GN: Enable embedder to decide
whether or not the external V8 snapshot is enabled."

https://codereview.chromium.org/749213004/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to