Reland https://codereview.chromium.org/2080733002

Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6 id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek

Committed: https://crrev.com/38ca936a5ddb9325607c5385aa7ec740cc842969
Cr-Commit-Position: refs/heads/master@{#400964}

[Miguel Garcia, 2016-06-20 18:24:30]

miguelg@chromium.org changed reviewers:
+ peter@chromium.org

[Miguel Garcia, 2016-06-20 18:24:30]

Just a sanity check review. I will TBR the original reviewers if you confirm we
are good.

[Peter Beverloo, 2016-06-20 18:54:24]

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:21:
'//ui/base:base',
This isn't how gyp works... Did you test it?

These have to be absolute references to the gyp files, relative from the path of
the current directory. For example:

<(DEPTH)/base/base.gyp:base
...

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:25: }
Repeated since you didn't apply it in the previous CL (please confirm before
uploading):

micro nits: (a) no blank line on :6, (b) 2-space indent on :7, (c) +2-space
indent elsewhere, (d) everything in gyp ends with a comma (:22)

[Miguel Garcia, 2016-06-20 19:17:30]

Actually I did. And it compiles.. will change it before landing though.
On 20 Jun 2016 7:54 p.m., <peter@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
> File chrome/browser/ui/cocoa/notifications/notification_common.gyp
> (right):
>
>
>
https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
> chrome/browser/ui/cocoa/notifications/notification_common.gyp:21:
> '//ui/base:base',
> This isn't how gyp works... Did you test it?
>
> These have to be absolute references to the gyp files, relative from the
> path of the current directory. For example:
>
> <(DEPTH)/base/base.gyp:base
> ...
>
>
>
https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
> chrome/browser/ui/cocoa/notifications/notification_common.gyp:25: }
> Repeated since you didn't apply it in the previous CL (please confirm
> before uploading):
>
> micro nits: (a) no blank line on :6, (b) 2-space indent on :7, (c)
> +2-space
> indent elsewhere, (d) everything in gyp ends with a comma (:22)
>
> https://codereview.chromium.org/2080603004/
>

-- 
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.

[Peter Beverloo, 2016-06-20 22:50:06]

That's odd, maybe it's transitional syntax. Please try to be consistent
nonetheless, and use the gyp syntax.

lgtm w/ that change, and the gyp indentation fixes

[Miguel Garcia, 2016-06-21 11:25:42]

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
File chrome/browser/ui/cocoa/notifications/notification_common.gyp (right):

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:21:
'//ui/base:base',
On 2016/06/20 18:54:23, Peter Beverloo wrote:
> This isn't how gyp works... Did you test it?
> 
> These have to be absolute references to the gyp files, relative from the path
of
> the current directory. For example:
> 
> <(DEPTH)/base/base.gyp:base
> ...

Done.

https://codereview.chromium.org/2080603004/diff/1/chrome/browser/ui/cocoa/not...
chrome/browser/ui/cocoa/notifications/notification_common.gyp:25: }
On 2016/06/20 18:54:23, Peter Beverloo wrote:
> Repeated since you didn't apply it in the previous CL (please confirm before
> uploading):
> 
> micro nits: (a) no blank line on :6, (b) 2-space indent on :7, (c) +2-space
> indent elsewhere, (d) everything in gyp ends with a comma (:22)

Done.

[Miguel Garcia, 2016-06-21 11:26:50]

Description was changed from

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
==========

to

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek
==========

[Miguel Garcia, 2016-06-21 11:26:50]

miguelg@chromium.org changed reviewers:
+ jochen@chromium.org, rsesek@chromium.org

[Miguel Garcia, 2016-06-21 11:26:55]

The CQ bit was checked by miguelg@chromium.org

[Miguel Garcia, 2016-06-21 11:26:56]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2080603004/#ps40001
(title: "format")

[commit-bot: I haz the power, 2016-06-21 11:27:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2080603004/40001

[commit-bot: I haz the power, 2016-06-21 12:27:22]

Description was changed from

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek
==========

to

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek
==========

[commit-bot: I haz the power, 2016-06-21 12:27:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-21 12:30:40]

Description was changed from

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek
==========

to

==========
Reland https://codereview.chromium.org/2080733002

Added missing deps in the gyp target.
Apparently only GN is run now as part of the CQ.

Revert "Revert of Make the notification builder more XPC friendly (patchset #6
id:100001 of https://codereview.chromium.org/2053613002/ )"

This reverts commit 9753d56a309ead6e186dc28dcd821f63c1c3e458.

BUG=571056
TBR=jochen,rsesek

Committed: https://crrev.com/38ca936a5ddb9325607c5385aa7ec740cc842969
Cr-Commit-Position: refs/heads/master@{#400964}
==========

[commit-bot: I haz the power, 2016-06-21 12:30:42]

Patchset 3 (id:??) landed as
https://crrev.com/38ca936a5ddb9325607c5385aa7ec740cc842969
Cr-Commit-Position: refs/heads/master@{#400964}