Run gn --format over all .gn files

Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

Exact command I ran:
find . -name "*.gn*" -exec gn format --in-place "{}" \;

TBR=ddorwin@chromium.org
BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/d7a71c88990b6788ceaf1ec9ee518099709ff3fd
Cr-Commit-Position: refs/heads/master@{#360891}

[agrieve, 2015-11-20 18:21:06]

Description was changed from

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[agrieve, 2015-11-20 18:21:06]

agrieve@chromium.org changed reviewers:
+ brettw@chromium.org

[agrieve, 2015-11-20 18:21:40]

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

[commit-bot: I haz the power, 2015-11-20 18:23:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1464873002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1464873002/1

[brettw, 2015-11-20 18:27:34]

lgtm

[agrieve, 2015-11-20 19:12:56]

Description was changed from

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

TBR=ddorwin@chromium.org
BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[kelvinp1, 2015-11-20 19:18:06]

kelvinp@google.com changed reviewers:
+ kelvinp@google.com

[kelvinp1, 2015-11-20 19:18:07]

lgtm

[kelvinp, 2015-11-20 19:18:50]

kelvinp@chromium.org changed reviewers:
+ kelvinp@chromium.org

[kelvinp, 2015-11-20 19:18:51]

lgtm

[tfarina, 2015-11-20 19:25:51]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2015-11-20 19:25:52]

lgtm! Looked at the first files briefly. Do you mind including in the CL
description the command line you used to reformat all the files? Did you scan
over .gni files as well?

https://codereview.chromium.org/1464873002/diff/1/android_webview/BUILD.gn
File android_webview/BUILD.gn (left):

https://codereview.chromium.org/1464873002/diff/1/android_webview/BUILD.gn#ol...
android_webview/BUILD.gn:103: "//content/app/resources",
humans are not good at sorting alphabetically themselves looks like, heh :)

https://codereview.chromium.org/1464873002/diff/1/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/ash/BUILD.gn#newcode276
ash/BUILD.gn:276: #"//chrome:packed_resources",  TODO(GYP)
I think we have this now. Let me take a look.

[agrieve, 2015-11-20 19:27:09]

Description was changed from

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

TBR=ddorwin@chromium.org
BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Run gn --format over all .gn files

The recent formatter alphebetizing change is causing a lot of noise in
code reviews. Figured it'd be worth a clean-up CL.

Exact command I ran:
find . -name "*.gn*" -exec gn format --in-place "{}" \;

TBR=ddorwin@chromium.org
BUG=554928
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[agrieve, 2015-11-20 19:28:47]

Added in the command line.

Thanks for taking a peek! (and for improving the formatter)!

https://codereview.chromium.org/1464873002/diff/1/android_webview/BUILD.gn
File android_webview/BUILD.gn (left):

https://codereview.chromium.org/1464873002/diff/1/android_webview/BUILD.gn#ol...
android_webview/BUILD.gn:103: "//content/app/resources",
On 2015/11/20 19:25:52, tfarina wrote:
> humans are not good at sorting alphabetically themselves looks like, heh :)

:)

[Kevin M, 2015-11-20 19:28:56]

kmarshall@chromium.org changed reviewers:
+ kmarshall@chromium.org

[Kevin M, 2015-11-20 19:28:57]

lgtm blimp

[tfarina, 2015-11-20 19:33:40]

https://codereview.chromium.org/1464873002/diff/1/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/ash/BUILD.gn#newcode276
ash/BUILD.gn:276: #"//chrome:packed_resources",  TODO(GYP)
On 2015/11/20 19:25:52, tfarina wrote:
> I think we have this now. Let me take a look.
Done. https://codereview.chromium.org/1465963002/

[commit-bot: I haz the power, 2015-11-20 19:38:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-20 19:38:08]

Dry run: This issue passed the CQ dry run.

[agrieve, 2015-11-20 19:41:29]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2015-11-20 19:42:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1464873002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1464873002/1

[commit-bot: I haz the power, 2015-11-20 19:53:38]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-11-20 19:54:24]

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

[Dirk Pranke, 2015-11-20 20:19:30]

I think this was a good thing to land, but we're probably gonna have to do this
again soon-ish because I think we got the ordering wrong :(.

[Dirk Pranke, 2015-11-20 20:26:34]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2015-11-20 20:26:37]

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
components/BUILD.gn:655: "strings",
specifically, this change looks worse.

[tfarina, 2015-11-20 20:35:49]

tfarina@chromium.org changed reviewers:
+ sdefresne@chromium.org

[tfarina, 2015-11-20 20:35:51]

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
components/BUILD.gn:655: "strings",
agreed. +sylvain!

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
components/BUILD.gn:655: "strings",
though this could have been //components/strings

[Dirk Pranke, 2015-11-20 20:41:52]

On 2015/11/20 20:35:51, tfarina wrote:
> https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn
> File components/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
> components/BUILD.gn:655: "strings",
> agreed. +sylvain!
> 
>
https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
> components/BUILD.gn:655: "strings",
> though this could have been //components/strings

Right. The style guide actually makes it pretty clear that it should be
"//components/strings"

https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/style_g...

but we don't forbid "strings" completely. Maybe we should; I can't think of a
reason why we'd
need to support it.

[brettw, 2015-11-20 21:19:52]

On 2015/11/20 20:41:52, Dirk Pranke wrote:
> but we don't forbid "strings" completely. Maybe we should; I can't think of a
> reason why we'd
> need to support it.

Some things like mojo wanted to be able to be pulled into chromium at a
different level than they are in their own repo, so all paths are relative. So
we need relative label names. We could force you to write "./strings" which
looks potentially less confusing, but this will be difficult to change due to
current usage.

[Dirk Pranke, 2015-11-20 21:27:56]

On 2015/11/20 21:19:52, brettw wrote:
> On 2015/11/20 20:41:52, Dirk Pranke wrote:
> > but we don't forbid "strings" completely. Maybe we should; I can't think of
a
> > reason why we'd
> > need to support it.
> 
> Some things like mojo wanted to be able to be pulled into chromium at a
> different level than they are in their own repo, so all paths are relative. So
> we need relative label names. We could force you to write "./strings" which
> looks potentially less confusing, but this will be difficult to change due to
> current usage.

Ah, that makes sense. 

"./strings" would look weirder than "strings", so
I'm fine w/ not changing it and just fixing the formatter.

[sdefresne, 2015-11-21 00:55:25]

Thank you for fixing this. I'll take a look at fixing the formatter soon and
re-run it over the whole codebase (hopefully there will be less churn as
relative path are not too frequent).

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
components/BUILD.gn:655: "strings",
On 2015/11/20 at 20:35:50, tfarina wrote:
> though this could have been //components/strings

Acknowledge, will fix the formatter soon, but here I think we should have used
//components/strings.

[sdefresne, 2015-11-21 01:38:56]

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1464873002/diff/1/components/BUILD.gn#newcode655
components/BUILD.gn:655: "strings",
On 2015/11/20 at 20:35:50, tfarina wrote:
> though this could have been //components/strings

https://codereview.chromium.org/1461143005

[tfarina, 2015-12-01 19:21:50]

On 2015/11/21 00:55:25, sdefresne wrote:
> Thank you for fixing this. I'll take a look at fixing the formatter soon and
> re-run it over the whole codebase (hopefully there will be less churn as
> relative path are not too frequent).
> 
Sylvain, did you get a time to look at this?

[sdefresne, 2015-12-02 18:43:24]

https://codereview.chromium.org/1496653002