Get rid of ui_cocoa_third_party_toolkits target from ui.gyp

Get rid of ui_cocoa_third_party_toolkits target from ui.gyp

Instead we add a gyp file in third_party/google_toolbox_for_mac to build
the sources from third_party/GTM. This is the third and last patch in
this series.

BUG=299841
TEST=None
R=mark@chromium.org,ben@chromium.org
TBR=ben

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

[tfarina, 2013-12-11 01:42:58]

Mark, I know you won't like this because it will sound a hack, but I couldn't
add to third_party/GTM.

$ g add third_party/GTM/GTM.gyp
fatal: Path 'third_party/GTM/GTM.gyp' is in submodule 'third_party/GTM'

[Mark Mentovai, 2013-12-11 02:23:23]

This means that we need to redo the way that we check out third_party/GTM. I
suggest creating third_party/google-toolbox-for-mac, and revising DEPS to check
out GTM into that directory. Put the .gyp file in
third_party/google-toolbox-for-mac too.

[tfarina, 2013-12-11 02:32:01]

On Wed, Dec 11, 2013 at 12:23 AM,  <mark@chromium.org> wrote:
> This means that we need to redo the way that we check out third_party/GTM. I
> suggest creating third_party/google-toolbox-for-mac, and revising DEPS to
> check
> out GTM into that directory. Put the .gyp file in
> third_party/google-toolbox-for-mac too.
>
You prefer dash instead of underscore? I already have
third_party/google_toolbox_for_mac. I'll rename to
google-toolbox-for-mac if that is your preference.

I'll contact who added third_party/GTM as submodule to see our options.

Thanks!

> https://codereview.chromium.org/107933006/



-- 
Thiago

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

[tfarina, 2013-12-11 02:38:52]

On Wed, Dec 11, 2013 at 12:31 AM, Thiago Farina <tfarina@chromium.org> wrote:
> On Wed, Dec 11, 2013 at 12:23 AM,  <mark@chromium.org> wrote:
>> This means that we need to redo the way that we check out third_party/GTM. I
>> suggest creating third_party/google-toolbox-for-mac, and revising DEPS to
>> check
>> out GTM into that directory. Put the .gyp file in
>> third_party/google-toolbox-for-mac too.
>>
> You prefer dash instead of underscore? I already have
> third_party/google_toolbox_for_mac. I'll rename to
> google-toolbox-for-mac if that is your preference.
>
Ah, to match with the "project" name like it is DEPS file and
code.google.com/p/google-toolbox-for-mac/?

-- 
Thiago

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

[Mark Mentovai, 2013-12-11 02:58:58]

I don’t care if it’s dash or underscore. I just typed dashes because that’s the
name of the project on code.google.com, but code.google.com doesn’t allow
underscores. I didn’t look at your change before I typed that. Underscores are
totally fine too.

[tfarina, 2013-12-12 23:11:30]

Let's see if this beast will compile.

Mark, please, take another look.

Thanks!

[Mark Mentovai, 2013-12-12 23:19:22]

https://codereview.chromium.org/107933006/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/107933006/diff/20001/DEPS#newcode67
DEPS:67: "src/third_party/google_toolbox_for_mac":
This was formerly in deps_os. Don’t move it into the universal deps.

https://codereview.chromium.org/107933006/diff/20001/DEPS#newcode67
DEPS:67: "src/third_party/google_toolbox_for_mac":
You want this to be
src/third_party/google_toolbox_for_mac/google_toolbox_for_mac or something.
Because you want to put your own file into
src/third_party/google_toolbox_for_mac. If you dump it into the same directory,
you’re stuck with the same problem we have now, where we can’t put our own files
into src/third_party/GTM. This will mean you’ll need to go back through and
readjust all of your #include changes.

https://codereview.chromium.org/107933006/diff/20001/DEPS#newcode69
DEPS:69: Var("google_toolbox_for_mac_revision"),
Blank line after, following the pattern of almost everything else in here. Well,
just make sure you fix it up when you move it to the right section. The blanks
are a little missingy down there.

[tfarina, 2013-12-12 23:24:09]

https://codereview.chromium.org/107933006/diff/20001/DEPS
File DEPS (right):

https://codereview.chromium.org/107933006/diff/20001/DEPS#newcode67
DEPS:67: "src/third_party/google_toolbox_for_mac":
On 2013/12/12 23:19:23, Mark Mentovai wrote:
> This was formerly in deps_os. Don’t move it into the universal deps.
Ops, this was me trying to test it here on my Linux machine.

Will move it back to its location.

[tfarina, 2013-12-13 12:05:23]

I think this is ready for another look. google_toolbox_for_mac linked in
mac_rel:

[98/7792] LIBTOOL-STATIC libgoogle_toolbox_for_mac.a, POSTBUILDS

[tfarina, 2013-12-13 12:05:23]

I think this is ready for another look. google_toolbox_for_mac linked in
mac_rel:

[98/7792] LIBTOOL-STATIC libgoogle_toolbox_for_mac.a, POSTBUILDS

[tfarina, 2013-12-13 12:10:06]

Not so fast:

Still have errors:

../../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h:19:10:
fatal error: 'GTMDefines.h' file not found

looking...

[tfarina, 2013-12-13 12:49:13]

Fixed.

This is ready (mac and mac_rel built) % licenses checks.

Submitting another try to see if it fixes the licenses check for android.

[Mark Mentovai, 2013-12-13 14:39:12]

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:49: # GTM is
third-party code, so we don't want to add _EXPORT
The indentation is messed up here.

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py
File tools/licenses.py (right):

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py#newco...
tools/licenses.py:93: os.path.join('third_party', 'google_toolbox_for_mac',
'src'): {
This line contains errors.

[tfarina, 2013-12-13 21:42:14]

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:49: # GTM is
third-party code, so we don't want to add _EXPORT
On 2013/12/13 14:39:13, Mark Mentovai wrote:
> The indentation is messed up here.

Done.

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py
File tools/licenses.py (right):

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py#newco...
tools/licenses.py:93: os.path.join('third_party', 'google_toolbox_for_mac',
'src'): {
On 2013/12/13 14:39:13, Mark Mentovai wrote:
> This line contains errors.
that is what I get for doing things while leaving home. Done.

[tfarina, 2013-12-13 21:42:14]

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/270001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:49: # GTM is
third-party code, so we don't want to add _EXPORT
On 2013/12/13 14:39:13, Mark Mentovai wrote:
> The indentation is messed up here.

Done.

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py
File tools/licenses.py (right):

https://codereview.chromium.org/107933006/diff/270001/tools/licenses.py#newco...
tools/licenses.py:93: os.path.join('third_party', 'google_toolbox_for_mac',
'src'): {
On 2013/12/13 14:39:13, Mark Mentovai wrote:
> This line contains errors.
that is what I get for doing things while leaving home. Done.

[Mark Mentovai, 2013-12-13 23:03:19]

https://codereview.chromium.org/107933006/diff/310001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/content_settings/collected_cookies_mac.mm:41: #include
"third_party/apple_sample_code/ImageAndTextCell.h"
Sort: google_toolbox_for_mac should follow apple_sample_code.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3316:
'../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h',
Shouldn’t this be built in the new GTM target now, making this target depend on
that one?

You should remove the 'sources' and 'includes' sections right here and replace
them with 'dependencies'.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
chrome/chrome_browser_ui.gypi:3174: '../third_party/google_toolbox_for_mac/src',
This shouldn’t be here. The new GTM target should publish this include_dirs
setting in its own direct_dependent_settings section. Dependent targetss
shouldn’t have to do this in order to use GTM.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gypi
File chrome/chrome_common.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gy...
chrome/chrome_common.gypi:542: '../third_party/google_toolbox_for_mac/src',
Same here. This target should depend on the new GTM target since it’s apparently
using it (chrome/common/mac/launchd.mm uses it). And this include_dirs setting
shouldn’t be here, but it should be published in a direct_dependent_settings
section in the new GTM target.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:1886: '../third_party/google_toolbox_for_mac/src',
Same here and line 2180. Depend on GTM, since it’s apparently necessary. Don’t
write GTM include_dirs into .gyp files anymore. That was a crutch that was
necessary because GTM wasn’t its own target. Where you find these now, it’s an
indication that the target that’s got this in its include_dirs is using GTM and
needs to depend on it.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2180: '../third_party/google_toolbox_for_mac/src',
Me too.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:2422: '../third_party/google_toolbox_for_mac/src',
Me too.

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp#n...
remoting/remoting.gyp:499:
'../third_party/google_toolbox_for_mac/src/AppKit/GTMCarbonEvent.h',
Don’t do this. Add the sources to the GTM target and depend on that instead.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:3: Version: unknown
Maybe this should be “see DEPS” or the specific revision we’re checking out,
instead of “unknown.”

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:5: License: Apache 2.0
See third_party/README.chromium.template

The end of this file should be like

--
License: Apache 2.0
License File: src/COPYING

Description:

A collection of source from different Google projects that may be of use to
developers working other Mac projects.

Local Modifications:

None.
--

I took the description from the GTM project page.

README.chromium.template also says to use a “Short Name,” I guess you’d use
google-toolbox-for-mac for that based on how I see other README.chromia using
that field.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:10:
'include_dirs': [
I think you will almost definitely need to also put all of these directories
into a 'direct_dependent_settings': {'include_dirs': […]} block. See my other
comments throughout the review.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:11:
'../../third_party/google_toolbox_for_mac',
You are in third_party/google_toolbox_for_mac. This can just be '.'.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:12:
'../../third_party/google_toolbox_for_mac/src',
And this can just be 'src'. Etc., etc.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:24:
'src/AppKit/GTMFadeTruncatingTextFieldCell.h',
Since this is its own target now, maybe you just want to list everything, or at
least everything in AppKit, DebugUtils, and Foundation. (I’m taking those names
based on what I see built and included here—lines 11-15 too. There’s also
src/AddressBook, which may also be harmless and useful to someone. If you add
src/AddressBook, don’t forget to add it to the include_dirs too.)

find src/AppKit src/DebugUtils src/Foundation -name '*.[c|cc|h|m|mm]' | sed -e
"s/\(.*\)/        '\1',/"

will format the list of all such files for pasting here.

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py
File tools/include_tracer.py (right):

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:28: 'third_party/google_toolbox_for_mac/src',
Sort. This belongs…

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:117: 'third_party/icu/public/common',
…down here now.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS
File ui/app_list/DEPS (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS#newcode7
ui/app_list/DEPS:7: "+third_party/google_toolbox_for_mac/src",
This list looks like it wanted to be sorted properly.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gyp
File ui/app_list/app_list.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gy...
ui/app_list/app_list.gyp:158: '../../third_party/google_toolbox_for_mac/src',
As discussed, don’t include_dirs here, just get it from
direct_dependent_settings.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
File ui/app_list/cocoa/signin_view_controller.mm (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
ui/app_list/cocoa/signin_view_controller.mm:11: #import
"third_party/google_toolbox_for_mac/src/AppKit/GTMUILocalizerAndLayoutTweaker.h"
Sort: g > s.

[Mark Mentovai, 2013-12-13 23:07:25]

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocols.h
File base/mac/cocoa_protocols.h (right):

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocol...
base/mac/cocoa_protocols.h:10: // GTM also maintinas a list of empty protocols,
but only the ones the library
Fix spelling while you’re in here: maintains.

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp
File breakpad/breakpad.gyp (right):

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp#n...
breakpad/breakpad.gyp:759: # For GTMLogger.
As discussed elsewhere, make this target depend on the GTM target instead of
messing with include_dirs and the really weird link_settings section that’s
here. Get rid of these three lines in this include_dirs, and get rid of the
entire link_settings section immediately following.

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS#newcode40
chrome/DEPS:40: "+third_party/google_toolbox_for_mac/src",
This portion of the list seems to be sorted.

[tfarina, 2013-12-16 02:42:02]

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3316:
'../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Shouldn’t this be built in the new GTM target now, making this target depend
on
> that one?
> 
> You should remove the 'sources' and 'includes' sections right here and replace
> them with 'dependencies'.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3316:
'../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Shouldn’t this be built in the new GTM target now, making this target depend
on
> that one?
> 
> You should remove the 'sources' and 'includes' sections right here and replace
> them with 'dependencies'.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
chrome/chrome_browser_ui.gypi:3174: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This shouldn’t be here. The new GTM target should publish this include_dirs
> setting in its own direct_dependent_settings section. Dependent targetss
> shouldn’t have to do this in order to use GTM.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gypi
File chrome/chrome_common.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gy...
chrome/chrome_common.gypi:542: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Same here. This target should depend on the new GTM target since it’s
apparently
> using it (chrome/common/mac/launchd.mm uses it). And this include_dirs setting
> shouldn’t be here, but it should be published in a direct_dependent_settings
> section in the new GTM target.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:1886: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Same here and line 2180. Depend on GTM, since it’s apparently necessary. Don’t
> write GTM include_dirs into .gyp files anymore. That was a crutch that was
> necessary because GTM wasn’t its own target. Where you find these now, it’s an
> indication that the target that’s got this in its include_dirs is using GTM
and
> needs to depend on it.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2180: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2180: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:2422: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp#n...
remoting/remoting.gyp:499:
'../third_party/google_toolbox_for_mac/src/AppKit/GTMCarbonEvent.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Don’t do this. Add the sources to the GTM target and depend on that instead.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:3: Version: unknown
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Maybe this should be “see DEPS” or the specific revision we’re checking out,
> instead of “unknown.”

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:5: License: Apache 2.0
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> See third_party/README.chromium.template
> 
> The end of this file should be like
> 
> --
> License: Apache 2.0
> License File: src/COPYING
> 
> Description:
> 
> A collection of source from different Google projects that may be of use to
> developers working other Mac projects.
> 
> Local Modifications:
> 
> None.
> --
> 
> I took the description from the GTM project page.
> 
> README.chromium.template also says to use a “Short Name,” I guess you’d use
> google-toolbox-for-mac for that based on how I see other README.chromia using
> that field.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:10:
'include_dirs': [
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> I think you will almost definitely need to also put all of these directories
> into a 'direct_dependent_settings': {'include_dirs': […]} block. See my other
> comments throughout the review.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:11:
'../../third_party/google_toolbox_for_mac',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> You are in third_party/google_toolbox_for_mac. This can just be '.'.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:12:
'../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> And this can just be 'src'. Etc., etc.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:24:
'src/AppKit/GTMFadeTruncatingTextFieldCell.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Since this is its own target now, maybe you just want to list everything, or
at
> least everything in AppKit, DebugUtils, and Foundation. (I’m taking those
names
> based on what I see built and included here—lines 11-15 too. There’s also
> src/AddressBook, which may also be harmless and useful to someone. If you add
> src/AddressBook, don’t forget to add it to the include_dirs too.)
> 
> find src/AppKit src/DebugUtils src/Foundation -name '*.[c|cc|h|m|mm]' | sed -e
> "s/\(.*\)/        '\1',/"
> 
> will format the list of all such files for pasting here.

Done.

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py
File tools/include_tracer.py (right):

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:28: 'third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Sort. This belongs…

Done.

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:117: 'third_party/icu/public/common',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> …down here now.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS
File ui/app_list/DEPS (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS#newcode7
ui/app_list/DEPS:7: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This list looks like it wanted to be sorted properly.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS#newcode7
ui/app_list/DEPS:7: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This list looks like it wanted to be sorted properly.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gyp
File ui/app_list/app_list.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gy...
ui/app_list/app_list.gyp:158: '../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> As discussed, don’t include_dirs here, just get it from
> direct_dependent_settings.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gy...
ui/app_list/app_list.gyp:158: '../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> As discussed, don’t include_dirs here, just get it from
> direct_dependent_settings.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
File ui/app_list/cocoa/signin_view_controller.mm (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
ui/app_list/cocoa/signin_view_controller.mm:11: #import
"third_party/google_toolbox_for_mac/src/AppKit/GTMUILocalizerAndLayoutTweaker.h"
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Sort: g > s.

Done.

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocols.h
File base/mac/cocoa_protocols.h (right):

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocol...
base/mac/cocoa_protocols.h:10: // GTM also maintinas a list of empty protocols,
but only the ones the library
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> Fix spelling while you’re in here: maintains.

Done.

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp
File breakpad/breakpad.gyp (right):

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp#n...
breakpad/breakpad.gyp:759: # For GTMLogger.
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> As discussed elsewhere, make this target depend on the GTM target instead of
> messing with include_dirs and the really weird link_settings section that’s
> here. Get rid of these three lines in this include_dirs, and get rid of the
> entire link_settings section immediately following.

Done.

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp#n...
breakpad/breakpad.gyp:759: # For GTMLogger.
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> As discussed elsewhere, make this target depend on the GTM target instead of
> messing with include_dirs and the really weird link_settings section that’s
> here. Get rid of these three lines in this include_dirs, and get rid of the
> entire link_settings section immediately following.

Done.

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS#newcode40
chrome/DEPS:40: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> This portion of the list seems to be sorted.

Done.

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS#newcode40
chrome/DEPS:40: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> This portion of the list seems to be sorted.

Done.

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:7: License File: src/COPYING
It complains, but the path seems right. 

Got LicenseError "License file not found. Either add a file named LICENSE,
import upstream's COPYING if available, or add a 'License File:' line to
README.chromium with the appropriate path." while scanning
third_party/google_toolbox_for_mac

http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/13...

[tfarina, 2013-12-16 02:42:03]

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3316:
'../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Shouldn’t this be built in the new GTM target now, making this target depend
on
> that one?
> 
> You should remove the 'sources' and 'includes' sections right here and replace
> them with 'dependencies'.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser.g...
chrome/chrome_browser.gypi:3316:
'../third_party/google_toolbox_for_mac/src/Foundation/GTMServiceManagement.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Shouldn’t this be built in the new GTM target now, making this target depend
on
> that one?
> 
> You should remove the 'sources' and 'includes' sections right here and replace
> them with 'dependencies'.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_browser_u...
chrome/chrome_browser_ui.gypi:3174: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This shouldn’t be here. The new GTM target should publish this include_dirs
> setting in its own direct_dependent_settings section. Dependent targetss
> shouldn’t have to do this in order to use GTM.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gypi
File chrome/chrome_common.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_common.gy...
chrome/chrome_common.gypi:542: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Same here. This target should depend on the new GTM target since it’s
apparently
> using it (chrome/common/mac/launchd.mm uses it). And this include_dirs setting
> shouldn’t be here, but it should be published in a direct_dependent_settings
> section in the new GTM target.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:1886: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Same here and line 2180. Depend on GTM, since it’s apparently necessary. Don’t
> write GTM include_dirs into .gyp files anymore. That was a crutch that was
> necessary because GTM wasn’t its own target. Where you find these now, it’s an
> indication that the target that’s got this in its include_dirs is using GTM
and
> needs to depend on it.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2180: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:2180: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/107933006/diff/310001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:2422: '../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp
File remoting/remoting.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/remoting/remoting.gyp#n...
remoting/remoting.gyp:499:
'../third_party/google_toolbox_for_mac/src/AppKit/GTMCarbonEvent.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Don’t do this. Add the sources to the GTM target and depend on that instead.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:3: Version: unknown
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Maybe this should be “see DEPS” or the specific revision we’re checking out,
> instead of “unknown.”

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:5: License: Apache 2.0
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> See third_party/README.chromium.template
> 
> The end of this file should be like
> 
> --
> License: Apache 2.0
> License File: src/COPYING
> 
> Description:
> 
> A collection of source from different Google projects that may be of use to
> developers working other Mac projects.
> 
> Local Modifications:
> 
> None.
> --
> 
> I took the description from the GTM project page.
> 
> README.chromium.template also says to use a “Short Name,” I guess you’d use
> google-toolbox-for-mac for that based on how I see other README.chromia using
> that field.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
File third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:10:
'include_dirs': [
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> I think you will almost definitely need to also put all of these directories
> into a 'direct_dependent_settings': {'include_dirs': […]} block. See my other
> comments throughout the review.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:11:
'../../third_party/google_toolbox_for_mac',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> You are in third_party/google_toolbox_for_mac. This can just be '.'.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:12:
'../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> And this can just be 'src'. Etc., etc.

Done.

https://codereview.chromium.org/107933006/diff/310001/third_party/google_tool...
third_party/google_toolbox_for_mac/google_toolbox_for_mac.gyp:24:
'src/AppKit/GTMFadeTruncatingTextFieldCell.h',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Since this is its own target now, maybe you just want to list everything, or
at
> least everything in AppKit, DebugUtils, and Foundation. (I’m taking those
names
> based on what I see built and included here—lines 11-15 too. There’s also
> src/AddressBook, which may also be harmless and useful to someone. If you add
> src/AddressBook, don’t forget to add it to the include_dirs too.)
> 
> find src/AppKit src/DebugUtils src/Foundation -name '*.[c|cc|h|m|mm]' | sed -e
> "s/\(.*\)/        '\1',/"
> 
> will format the list of all such files for pasting here.

Done.

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py
File tools/include_tracer.py (right):

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:28: 'third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Sort. This belongs…

Done.

https://codereview.chromium.org/107933006/diff/310001/tools/include_tracer.py...
tools/include_tracer.py:117: 'third_party/icu/public/common',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> …down here now.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS
File ui/app_list/DEPS (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS#newcode7
ui/app_list/DEPS:7: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This list looks like it wanted to be sorted properly.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/DEPS#newcode7
ui/app_list/DEPS:7: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> This list looks like it wanted to be sorted properly.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gyp
File ui/app_list/app_list.gyp (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gy...
ui/app_list/app_list.gyp:158: '../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> As discussed, don’t include_dirs here, just get it from
> direct_dependent_settings.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/app_list.gy...
ui/app_list/app_list.gyp:158: '../../third_party/google_toolbox_for_mac/src',
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> As discussed, don’t include_dirs here, just get it from
> direct_dependent_settings.

Done.

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
File ui/app_list/cocoa/signin_view_controller.mm (right):

https://codereview.chromium.org/107933006/diff/310001/ui/app_list/cocoa/signi...
ui/app_list/cocoa/signin_view_controller.mm:11: #import
"third_party/google_toolbox_for_mac/src/AppKit/GTMUILocalizerAndLayoutTweaker.h"
On 2013/12/13 23:03:20, Mark Mentovai wrote:
> Sort: g > s.

Done.

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocols.h
File base/mac/cocoa_protocols.h (right):

https://codereview.chromium.org/107933006/diff/330001/base/mac/cocoa_protocol...
base/mac/cocoa_protocols.h:10: // GTM also maintinas a list of empty protocols,
but only the ones the library
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> Fix spelling while you’re in here: maintains.

Done.

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp
File breakpad/breakpad.gyp (right):

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp#n...
breakpad/breakpad.gyp:759: # For GTMLogger.
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> As discussed elsewhere, make this target depend on the GTM target instead of
> messing with include_dirs and the really weird link_settings section that’s
> here. Get rid of these three lines in this include_dirs, and get rid of the
> entire link_settings section immediately following.

Done.

https://codereview.chromium.org/107933006/diff/330001/breakpad/breakpad.gyp#n...
breakpad/breakpad.gyp:759: # For GTMLogger.
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> As discussed elsewhere, make this target depend on the GTM target instead of
> messing with include_dirs and the really weird link_settings section that’s
> here. Get rid of these three lines in this include_dirs, and get rid of the
> entire link_settings section immediately following.

Done.

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS
File chrome/DEPS (right):

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS#newcode40
chrome/DEPS:40: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> This portion of the list seems to be sorted.

Done.

https://codereview.chromium.org/107933006/diff/330001/chrome/DEPS#newcode40
chrome/DEPS:40: "+third_party/google_toolbox_for_mac/src",
On 2013/12/13 23:07:26, Mark Mentovai wrote:
> This portion of the list seems to be sorted.

Done.

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:7: License File: src/COPYING
It complains, but the path seems right. 

Got LicenseError "License file not found. Either add a file named LICENSE,
import upstream's COPYING if available, or add a 'License File:' line to
README.chromium with the appropriate path." while scanning
third_party/google_toolbox_for_mac

http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/13...

[tfarina, 2013-12-16 23:04:20]

+Richard for the android_webview license issue.

Richard, do you know what I'm doing wrong? Why is it complaining? Please, see
third_party/google_toolbox_for_mac/

[Mark Mentovai, 2013-12-16 23:06:00]

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:7: License File: src/COPYING
src/COPYING is checked out in a deps_os section on Mac only. Is the license
checker running on Linux?

[tfarina, 2013-12-16 23:36:40]

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/510001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:7: License File: src/COPYING
On 2013/12/16 23:06:01, Mark Mentovai wrote:
> src/COPYING is checked out in a deps_os section on Mac only.
yep.

> Is the license checker running on Linux?
yep.

The problem seems to be that the directory and thus README.chromium and the gyp
file are present in non-mac checkouts. Because they are checked in the source
tree rather than being checkout out from upstream. And the license tool scans
the directories in third_party, so it finds us on Linux but do not find the
src/COPYING.

[tfarina, 2013-12-17 02:53:48]

Mark I added a LICENSE file, copied from
chrome/installer/mac/third_party/xz/LICENSE

I'm hoping that will silence the license check.

PTAL.

[Torne, 2013-12-17 11:16:54]

On 2013/12/17 02:53:48, tfarina wrote:
> Mark I added a LICENSE file, copied from
> chrome/installer/mac/third_party/xz/LICENSE
> 
> I'm hoping that will silence the license check.
> 
> PTAL.

The LICENSE file is only used for the check if you don't specify a license file
in README.chromium - either delete the line or change it to refer to the file in
the main repository.

Sorry about this but unfortunately the various license checks run on different
bots that check out different parts of the source :)

[tfarina, 2013-12-17 12:17:05]

On 2013/12/17 11:16:54, Torne wrote:
> The LICENSE file is only used for the check if you don't specify a license
file
> in README.chromium - either delete the line or change it to refer to the file
in
> the main repository.
I removed the "License File:" line from the README.chromium as suggested.

[Torne, 2013-12-17 14:58:27]

On 2013/12/17 12:17:05, tfarina wrote:
> On 2013/12/17 11:16:54, Torne wrote:
> > The LICENSE file is only used for the check if you don't specify a license
> file
> > in README.chromium - either delete the line or change it to refer to the
file
> in
> > the main repository.
> I removed the "License File:" line from the README.chromium as suggested.

OK, thanks for this. The licensing stuff looks fine to me and the bot seems
happy. Sorry our weird requirements are such a pain :/

[Mark Mentovai, 2013-12-17 15:19:08]

The bots like it and so do I. LGTM.

https://codereview.chromium.org/107933006/diff/580001/.gitignore
File .gitignore (right):

https://codereview.chromium.org/107933006/diff/580001/.gitignore#newcode218
.gitignore:218: /third_party/google_toolbox_for_mac/src
Be sure to deal with svn:ignore for this too. You may need to do it in a
separate followup because git doesn’t deal with Subversion properties.

https://codereview.chromium.org/107933006/diff/580001/remoting/DEPS
File remoting/DEPS (right):

https://codereview.chromium.org/107933006/diff/580001/remoting/DEPS#newcode14
remoting/DEPS:14: "+third_party/google_toolbox_for_mac/src/AppKit",
It should be fine for this to depend on anything in
third_party/google_toolbox_for_mac/src. It should work if you remove the /AppKit
from this line. There’s no good reason to restrict this to just things in the
AppKit subdirectory.

https://codereview.chromium.org/107933006/diff/580001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/580001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:4: Version: 616
This is likely to get out of sync with the DEPS file. It’s OK, but I wonder if
“see DEPS” would be better.

[commit-bot: I haz the power, 2013-12-18 00:41:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/107933006/600001

[tfarina, 2013-12-18 00:42:55]

https://codereview.chromium.org/107933006/diff/580001/remoting/DEPS
File remoting/DEPS (right):

https://codereview.chromium.org/107933006/diff/580001/remoting/DEPS#newcode14
remoting/DEPS:14: "+third_party/google_toolbox_for_mac/src/AppKit",
On 2013/12/17 15:19:09, Mark Mentovai wrote:
> It should be fine for this to depend on anything in
> third_party/google_toolbox_for_mac/src. It should work if you remove the
/AppKit
> from this line. There’s no good reason to restrict this to just things in the
> AppKit subdirectory.

Done.

https://codereview.chromium.org/107933006/diff/580001/third_party/google_tool...
File third_party/google_toolbox_for_mac/README.chromium (right):

https://codereview.chromium.org/107933006/diff/580001/third_party/google_tool...
third_party/google_toolbox_for_mac/README.chromium:4: Version: 616
On 2013/12/17 15:19:09, Mark Mentovai wrote:
> This is likely to get out of sync with the DEPS file. It’s OK, but I wonder if
> “see DEPS” would be better.
I had to keep this, otherwise:

** Presubmit ERRORS **
Third party README files should contain a 'Version' field.
If the package is not versioned or the version is not known
list the version as 'unknown'.
Check README.chromium.template for details.
  third_party/google_toolbox_for_mac/README.chromium

[commit-bot: I haz the power, 2013-12-18 03:58:41]

Change committed as 241471