Fix chrome/browser/ui dependency on components/translate

Fix chrome/browser/ui dependency on components/translate

It needs to be a public dependency as it's exposed by
chrome/browser/ui/browser.h and uses generated code.

BUG=649766
R=thakis@chromium.org
TBR=ben@chromium.org

Committed: https://crrev.com/387131a7eec324495c917bbd5317c3659a487011
Cr-Commit-Position: refs/heads/master@{#420717}

[Ken Rockot(use gerrit already), 2016-09-23 19:14:31]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 19:14:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2016-09-23 19:16:24]

lgtm

This happens all the time with mojo code :-/

[Ken Rockot(use gerrit already), 2016-09-23 19:21:23]

Yes but the underlying problem is that very few developers seem to
understand the difference between deps and public_deps and so very few
developers get it right. This mostly manifests as mojom compile errors
because getting the wrong public_deps only really breaks things when
configs or generated files are involved.

What we really need is for GN check to understand public_deps and enforce
their proper use, but that seems like a hard problem. IIRC Brett punted on
it previously.

On Fri, Sep 23, 2016 at 12:16 PM, <thakis@chromium.org> wrote:

> lgtm
>
> This happens all the time with mojo code :-/
>
> https://codereview.chromium.org/2368633002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-09-23 19:22:26]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-09-23 19:22:28]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-09-23 19:23:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nico, 2016-09-23 19:24:48]

Yeah, but generated header files have been a known broken thing since
forever (protoc writes them too), and I suggested that we should probably
solve this problem before adopting mojo widely. That was ignored, and
predictably this is now broken all the time :-/

On Fri, Sep 23, 2016 at 3:21 PM, Ken Rockot <rockot@chromium.org> wrote:

> Yes but the underlying problem is that very few developers seem to
> understand the difference between deps and public_deps and so very few
> developers get it right. This mostly manifests as mojom compile errors
> because getting the wrong public_deps only really breaks things when
> configs or generated files are involved.
>
> What we really need is for GN check to understand public_deps and enforce
> their proper use, but that seems like a hard problem. IIRC Brett punted on
> it previously.
>
> On Fri, Sep 23, 2016 at 12:16 PM, <thakis@chromium.org> wrote:
>
>> lgtm
>>
>> This happens all the time with mojo code :-/
>>
>> https://codereview.chromium.org/2368633002/
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-09-23 19:44:33]

I don't think it's fair to claim your suggestion was ignored so much as
there's disagreement about whether this should be treated as a blocking
issue.

It would certainly be nice to have checks to help people get their deps
right, but the impact does not seem to be as detrimental as your language
would imply unless I'm missing some evidence to the contrary.

On Fri, Sep 23, 2016 at 12:24 PM, Nico Weber <thakis@chromium.org> wrote:

> Yeah, but generated header files have been a known broken thing since
> forever (protoc writes them too), and I suggested that we should probably
> solve this problem before adopting mojo widely. That was ignored, and
> predictably this is now broken all the time :-/
>
> On Fri, Sep 23, 2016 at 3:21 PM, Ken Rockot <rockot@chromium.org> wrote:
>
>> Yes but the underlying problem is that very few developers seem to
>> understand the difference between deps and public_deps and so very few
>> developers get it right. This mostly manifests as mojom compile errors
>> because getting the wrong public_deps only really breaks things when
>> configs or generated files are involved.
>>
>> What we really need is for GN check to understand public_deps and enforce
>> their proper use, but that seems like a hard problem. IIRC Brett punted on
>> it previously.
>>
>> On Fri, Sep 23, 2016 at 12:16 PM, <thakis@chromium.org> wrote:
>>
>>> lgtm
>>>
>>> This happens all the time with mojo code :-/
>>>
>>> https://codereview.chromium.org/2368633002/
>>>
>>
>>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-09-23 20:31:19]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-23 20:32:57]

Description was changed from

==========
Fix chrome/browser/ui dependency on components/translate

It needs to be a public dependency as it's exposed by
chrome/browser/ui/browser.h and uses generated code.

BUG=649766
R=thakis@chromium.org
TBR=ben@chromium.org
==========

to

==========
Fix chrome/browser/ui dependency on components/translate

It needs to be a public dependency as it's exposed by
chrome/browser/ui/browser.h and uses generated code.

BUG=649766
R=thakis@chromium.org
TBR=ben@chromium.org

Committed: https://crrev.com/387131a7eec324495c917bbd5317c3659a487011
Cr-Commit-Position: refs/heads/master@{#420717}
==========

[commit-bot: I haz the power, 2016-09-23 20:32:58]

Patchset 1 (id:??) landed as
https://crrev.com/387131a7eec324495c917bbd5317c3659a487011
Cr-Commit-Position: refs/heads/master@{#420717}

[Nico, 2016-09-23 20:38:02]

On Fri, Sep 23, 2016 at 3:44 PM, Ken Rockot <rockot@chromium.org> wrote:

> I don't think it's fair to claim your suggestion was ignored so much as
> there's disagreement about whether this should be treated as a blocking
> issue.
>
> It would certainly be nice to have checks to help people get their deps
> right, but the impact does not seem to be as detrimental as your language
> would imply unless I'm missing some evidence to the contrary.
>

I don't keep a record, but it feels like the tree breaks due to this
problem every 1-2 weeks, which is unusually common for a single problem
cause.


>
> On Fri, Sep 23, 2016 at 12:24 PM, Nico Weber <thakis@chromium.org> wrote:
>
>> Yeah, but generated header files have been a known broken thing since
>> forever (protoc writes them too), and I suggested that we should probably
>> solve this problem before adopting mojo widely. That was ignored, and
>> predictably this is now broken all the time :-/
>>
>> On Fri, Sep 23, 2016 at 3:21 PM, Ken Rockot <rockot@chromium.org> wrote:
>>
>>> Yes but the underlying problem is that very few developers seem to
>>> understand the difference between deps and public_deps and so very few
>>> developers get it right. This mostly manifests as mojom compile errors
>>> because getting the wrong public_deps only really breaks things when
>>> configs or generated files are involved.
>>>
>>> What we really need is for GN check to understand public_deps and
>>> enforce their proper use, but that seems like a hard problem. IIRC Brett
>>> punted on it previously.
>>>
>>> On Fri, Sep 23, 2016 at 12:16 PM, <thakis@chromium.org> wrote:
>>>
>>>> lgtm
>>>>
>>>> This happens all the time with mojo code :-/
>>>>
>>>> https://codereview.chromium.org/2368633002/
>>>>
>>>
>>>
>>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.