Cleanup: Forward declare v8::Isolate in V8Initializer.

Cleanup: Forward declare v8::Isolate in V8Initializer.

We can forward declare it because we are not calling any function of
Isolate in the header, so the forward declaration is enough to make a
pointer declaration and thus we don't need to include v8.h in the
header.

BUG=None
TEST=None
R=abarth@chromium.org

[abarth-chromium, 2014-02-15 19:07:14]

not lgtm

This CL doesn't solve any problem.  It's perfectly normal for code in
bindings/v8 to include v8.h

[tfarina, 2014-02-16 14:10:53]

On 2014/02/15 19:07:14, abarth wrote:
> not lgtm
> 
> This CL doesn't solve any problem.  It's perfectly normal for code in
> bindings/v8 to include v8.h

[tfarina, 2014-02-16 14:12:06]

Adam, please, could you read the first section of
http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I think that
covers the good practice here.

[haraken, 2014-02-16 14:35:45]

On 2014/02/16 14:12:06, tfarina wrote:
> Adam, please, could you read the first section of
> http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I think
that
> covers the good practice here.

not lgtm2.

In a particular case of v8.h, there is no benefit in replacing #include with
forward declarations. As Adam mentioned, it's normal for v8 bindings to include
v8.h to avoid writing a lot of forward declarations.

[tfarina, 2014-02-16 14:43:57]

On 2014/02/16 14:35:45, haraken wrote:
> On 2014/02/16 14:12:06, tfarina wrote:
> > Adam, please, could you read the first section of
> > http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I think
> that
> > covers the good practice here.
> 
> not lgtm2.
> 
> In a particular case of v8.h, there is no benefit in replacing #include with
> forward declarations. As Adam mentioned, it's normal for v8 bindings to
include
> v8.h to avoid writing a lot of forward declarations.

[tfarina, 2014-02-16 14:43:57]

On 2014/02/16 14:35:45, haraken wrote:
> On 2014/02/16 14:12:06, tfarina wrote:
> > Adam, please, could you read the first section of
> > http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I think
> that
> > covers the good practice here.
> 
> not lgtm2.
> 
> In a particular case of v8.h, there is no benefit in replacing #include with
> forward declarations. As Adam mentioned, it's normal for v8 bindings to
include
> v8.h to avoid writing a lot of forward declarations.

[tfarina, 2014-02-16 14:44:41]

OK, I guess it is fine, since v8.h does not include other headers.

Closing this. Thanks for the review.

[tfarina, 2014-02-16 14:48:42]

On Sun, Feb 16, 2014 at 11:35 AM, <haraken@chromium.org> wrote:

> On 2014/02/16 14:12:06, tfarina wrote:
>
>> Adam, please, could you read the first section of
>> http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I
>> think
>>
> that
>
>> covers the good practice here.
>>
>
> not lgtm2.
>
> In a particular case of v8.h, there is no benefit in replacing #include
> with
> forward declarations. As Adam mentioned, it's normal for v8 bindings to
> include
> v8.h to avoid writing a lot of forward declarations.
>
> Writing (a lot of) forward declarations is still better than including
(unnecessary) headers in other headers. The main reason is compilation
time, and to be honest WebKit/Blink has the worst compilation time among
the projects used on Chromium, and since I have a slow machine that matters
to me. It certainly does not matter when you have a z620 or a cluster to
build your binary under 2 minutes.

-- 
Thiago

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[abarth-chromium, 2014-02-18 01:18:23]

On Sun, Feb 16, 2014 at 6:48 AM, Thiago Farina <tfarina@chromium.org> wrote:

> On Sun, Feb 16, 2014 at 11:35 AM, <haraken@chromium.org> wrote:
>
>> On 2014/02/16 14:12:06, tfarina wrote:
>>
>>> Adam, please, could you read the first section of
>>> http://www.chromium.org/developers/coding-style/cpp-dos-and-donts? I
>>> think
>>>
>> that
>>
>>> covers the good practice here.
>>>
>>
>> not lgtm2.
>>
>> In a particular case of v8.h, there is no benefit in replacing #include
>> with
>> forward declarations. As Adam mentioned, it's normal for v8 bindings to
>> include
>> v8.h to avoid writing a lot of forward declarations.
>>
>> Writing (a lot of) forward declarations is still better than including
> (unnecessary) headers in other headers. The main reason is compilation
> time, and to be honest WebKit/Blink has the worst compilation time among
> the projects used on Chromium, and since I have a slow machine that matters
> to me. It certainly does not matter when you have a z620 or a cluster to
> build your binary under 2 minutes.
>

I'm in favor of working to improve compilation time.  If you have data
about how much we can improve compilation time in this way, I'd be
interested to see it.

Adam

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.