Remove static initializers introduced by open-vcdiff's logging.h.

Remove static initializers introduced by open-vcdiff's logging.h.

I wanted to change open-vcdiff's logging.h directly, but we pull it directly
from google code, not through deps, so we can't have downstream diffs.

Maybe that's enough to eliminate all static initializers added by open-vcdiff.

BUG=94925
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234746

[Nico, 2013-11-12 00:36:11]

err, s/willchan/wtc/. For some reason I thought willchan was an owner here, but
he isn't.

[wtc, 2013-11-12 02:17:39]

Patch set 10 LGTM. Please give jar two days (until the end of Wednesday) to take
a look at this CL. Thanks.

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (left):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#oldcode35
sdch/sdch.gyp:35: 'open-vcdiff/src/logging.h',

Please confirm that you don't want to remove these two files from the source
tree.

Why don't we just replace open-vcdiff/src/logging.h with the contents of
logging_forward.h?

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode48
sdch/sdch.gyp:48: '..',

Is this pointing to the src/sdch directory? Why isn't this '.'?

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode70
sdch/sdch.gyp:70: 'OPEN_VCDIFF_LOGGING_H_',

Why don't we define this macro in logging_forward.h?

[wtc, 2013-11-12 02:19:29]

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (left):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#oldcode35
sdch/sdch.gyp:35: 'open-vcdiff/src/logging.h',

On 2013/11/12 02:17:39, wtc wrote:
>
> Why don't we just replace open-vcdiff/src/logging.h with the contents of
> logging_forward.h?

Please ignore this question. I found the answer in the CL's description.

[Nico, 2013-11-12 03:34:10]

Thanks!

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode48
sdch/sdch.gyp:48: '..',
On 2013/11/12 02:17:39, wtc wrote:
> 
> Is this pointing to the src/sdch directory? Why isn't this '.'?

It's pointing to src/, so that the include of "base/logging.h" succeeds.

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode70
sdch/sdch.gyp:70: 'OPEN_VCDIFF_LOGGING_H_',
On 2013/11/12 02:17:39, wtc wrote:
> 
> Why don't we define this macro in logging_forward.h?

That works too. I put it here so that it's next to the comment that adds the
forced include. If you prefer, I can move it into the .h file (there's a comment
like this too after all).

…actually, since you asked, you probably do prefer that, so done :-)

[wtc, 2013-11-12 18:14:02]

Patch set 11 LGTM. Thanks.

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode48
sdch/sdch.gyp:48: '..',

On 2013/11/12 03:34:10, Nico wrote:
>
> It's pointing to src/, so that the include of "base/logging.h" succeeds.

Thank you. This was not obvious to me, so it might be worth a comment.

Also, should this be done by adding a dependency on '../base/base.gyp:base'
instead? base/base.gyp:base has this:

      'direct_dependent_settings': {
        'include_dirs': [
          '..',
        ],
      },

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h
File sdch/logging_forward.h (right):

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h#n...
sdch/logging_forward.h:17: #define VCD_ENDL "\n"

This is missing the open_vcdiff::CheckFatalError() call in the original code. I
think it is OK because our LOG(DFATAL) should provide the equivalent
functionality.

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h#n...
sdch/logging_forward.h:20: #define OPEN_VCDIFF_LOGGING_H_

Nit: it seems a little better to define this header guard right below line 6, so
that both header guards are side by side.

https://codereview.chromium.org/68253006/diff/370001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/370001/sdch/sdch.gyp#newcode66
sdch/sdch.gyp:66: # the header guard so that open-vcdiff's logging.h doesn't get
used, and

Remove "Define the header guard so that open-vcdiff's logging.h doesn't get
used" from this comment.

[Nico, 2013-11-12 18:34:41]

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/280001/sdch/sdch.gyp#newcode48
sdch/sdch.gyp:48: '..',
On 2013/11/12 18:14:03, wtc wrote:
> 
> On 2013/11/12 03:34:10, Nico wrote:
> >
> > It's pointing to src/, so that the include of "base/logging.h" succeeds.
> 
> Thank you. This was not obvious to me, so it might be worth a comment.
> 
> Also, should this be done by adding a dependency on '../base/base.gyp:base'
> instead? base/base.gyp:base has this:
> 
>       'direct_dependent_settings': {
>         'include_dirs': [
>           '..',
>         ],
>       },

Makes sense, done.

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h
File sdch/logging_forward.h (right):

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h#n...
sdch/logging_forward.h:17: #define VCD_ENDL "\n"
On 2013/11/12 18:14:03, wtc wrote:
> 
> This is missing the open_vcdiff::CheckFatalError() call in the original code.
I
> think it is OK because our LOG(DFATAL) should provide the equivalent
> functionality.

Yes. (I think LOG(DFATAL) might implicitly add a newline too, so just "" might
be enough too. Not sure though, and I suppose it doesn't matter much either
way.)

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h#n...
sdch/logging_forward.h:20: #define OPEN_VCDIFF_LOGGING_H_
On 2013/11/12 18:14:03, wtc wrote:
> 
> Nit: it seems a little better to define this header guard right below line 6,
so
> that both header guards are side by side.

Done.

https://codereview.chromium.org/68253006/diff/370001/sdch/sdch.gyp
File sdch/sdch.gyp (right):

https://codereview.chromium.org/68253006/diff/370001/sdch/sdch.gyp#newcode66
sdch/sdch.gyp:66: # the header guard so that open-vcdiff's logging.h doesn't get
used, and
On 2013/11/12 18:14:03, wtc wrote:
> 
> Remove "Define the header guard so that open-vcdiff's logging.h doesn't get
> used" from this comment.

Done.

[jar (doing other things), 2013-11-13 00:34:45]

Nice workaround.

+1 for Want-eh's review, and then an agreeable (rubber stamp) LGTM.

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h
File sdch/logging_forward.h (right):

https://codereview.chromium.org/68253006/diff/370001/sdch/logging_forward.h#n...
sdch/logging_forward.h:20: #define OPEN_VCDIFF_LOGGING_H_
On 2013/11/12 18:14:03, wtc wrote:
> 
> Nit: it seems a little better to define this header guard right below line 6,
so
> that both header guards are side by side.

+1

[jar (doing other things), 2013-11-13 00:37:24]

(this time... with correct spelling of a name.....)

Nice workaround.

+1 for Wan-teh's review, and then an agreeable (rubber stamp) LGTM.

[wtc, 2013-11-13 00:52:56]

Patch set 12 LGTM.

[commit-bot: I haz the power, 2013-11-13 01:02:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/68253006/490001

[commit-bot: I haz the power, 2013-11-13 04:11:51]

Change committed as 234746