Make new target for vector icon structs to replace ui/gfx/vector_icons/

Make new target for vector icon structs to replace
ui/gfx/vector_icons/

Migrate just one icon over for now.

BUG=626786
Review-Url: https://codereview.chromium.org/2667753006
Cr-Commit-Position: refs/heads/master@{#448789}
Committed: https://chromium.googlesource.com/chromium/src/+/8741905369e3debd9ae7e6eb469fa47a60fbd9b6

[Evan Stade, 2017-02-01 02:48:38]

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

[commit-bot: I haz the power, 2017-02-01 02:48:57]

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

[commit-bot: I haz the power, 2017-02-01 02:58:51]

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

[commit-bot: I haz the power, 2017-02-01 02:58:52]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2017-02-01 15:55:28]

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

[commit-bot: I haz the power, 2017-02-01 15:55:46]

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

[commit-bot: I haz the power, 2017-02-01 16:07:13]

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

[commit-bot: I haz the power, 2017-02-01 16:07:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2017-02-01 22:10:10]

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

[commit-bot: I haz the power, 2017-02-01 22:10:30]

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

[commit-bot: I haz the power, 2017-02-01 23:12:58]

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

[commit-bot: I haz the power, 2017-02-01 23:12:59]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2017-02-03 01:34:24]

Description was changed from

==========
WIP - make new target for vector icon structs to replace
ui/gfx/vector_icons/

probably needs exportage or to be a component?

BUG=626786
==========

to

==========
Make new target for vector icon structs to replace
ui/gfx/vector_icons/

Migrate just one icon over for now.

BUG=626786
==========

[Evan Stade, 2017-02-03 01:34:24]

estade@chromium.org changed reviewers:
+ sky@chromium.org

[sky, 2017-02-03 16:20:06]

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.gn
File ui/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:1: # Copyright 2016 The Chromium Authors. All rights
reserved.
2017

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:10: icons = [
Beyond the icons, is there anything different between this and the other similar
files? Can you use a template to minimize the copying?

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/vector_...
File ui/vector_icons/vector_icons.h.template (right):

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/vector_...
ui/vector_icons/vector_icons.h.template:16: extern const gfx::VectorIcon
icon_name;
Don't you need to export there?

[Evan Stade, 2017-02-03 18:35:00]

estade@chromium.org changed reviewers:
+ thakis@chromium.org

[Evan Stade, 2017-02-03 18:35:01]

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.gn
File ui/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:1: # Copyright 2016 The Chromium Authors. All rights
reserved.
On 2017/02/03 16:20:06, sky wrote:
> 2017

Done.

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:10: icons = [
On 2017/02/03 16:20:06, sky wrote:
> Beyond the icons, is there anything different between this and the other
similar
> files? Can you use a template to minimize the copying?

Yes, this stanza is a bit repetitive, but more often than not it's part of
another target instead of its own file. I don't know how to cut down on the
repetition. Perhaps  we can add a template like grit() (defined in
//tools/grit/grit_rule.gni)?

+thakis to weigh in

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/vector_...
File ui/vector_icons/vector_icons.h.template (right):

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/vector_...
ui/vector_icons/vector_icons.h.template:16: extern const gfx::VectorIcon
icon_name;
On 2017/02/03 16:20:06, sky wrote:
> Don't you need to export there?

I would have thought so, but I believe that only applies to components (this is
a shared library).

"""
If your code is not being compiled into a component, don’t define such a macro
(sometimes people do this by copying other targets without understanding).
"""
https://chromium.googlesource.com/chromium/src/+/master/docs/component_build.md

There is test coverage for this --- dbg windows trybots would have complained if
it needed an export.

[Evan Stade, 2017-02-03 18:35:09]

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

[commit-bot: I haz the power, 2017-02-03 18:35:35]

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

[Nico, 2017-02-03 18:40:16]

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.gn
File ui/vector_icons/BUILD.gn (right):

https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
ui/vector_icons/BUILD.gn:10: icons = [
On 2017/02/03 18:35:01, Evan Stade wrote:
> On 2017/02/03 16:20:06, sky wrote:
> > Beyond the icons, is there anything different between this and the other
> similar
> > files? Can you use a template to minimize the copying?
> 
> Yes, this stanza is a bit repetitive, but more often than not it's part of
> another target instead of its own file. I don't know how to cut down on the
> repetition. Perhaps  we can add a template like grit() (defined in
> //tools/grit/grit_rule.gni)?
> 
> +thakis to weigh in

It's possible to use a template for this, if that's the question :-) There are
many examples, several pretty small:
https://cs.chromium.org/search/?q=template%5C(+file:%5C.gn+package:%5Echromiu...

[Evan Stade, 2017-02-03 18:43:43]

On 2017/02/03 18:40:16, Nico wrote:
> https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.gn
> File ui/vector_icons/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2667753006/diff/40001/ui/vector_icons/BUILD.g...
> ui/vector_icons/BUILD.gn:10: icons = [
> On 2017/02/03 18:35:01, Evan Stade wrote:
> > On 2017/02/03 16:20:06, sky wrote:
> > > Beyond the icons, is there anything different between this and the other
> > similar
> > > files? Can you use a template to minimize the copying?
> > 
> > Yes, this stanza is a bit repetitive, but more often than not it's part of
> > another target instead of its own file. I don't know how to cut down on the
> > repetition. Perhaps  we can add a template like grit() (defined in
> > //tools/grit/grit_rule.gni)?
> > 
> > +thakis to weigh in
> 
> It's possible to use a template for this, if that's the question :-) There are
> many examples, several pretty small:
>
https://cs.chromium.org/search/?q=template%5C(+file:%5C.gn+package:%5Echromiu...

Thanks. Scott, ok if I make following up on this a TODO?

[commit-bot: I haz the power, 2017-02-03 20:10:01]

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

[commit-bot: I haz the power, 2017-02-03 20:10:02]

Dry run: This issue passed the CQ dry run.

[sky, 2017-02-03 23:35:57]

LGTM - put please use a template soonish.

[Evan Stade, 2017-02-06 16:37:07]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-02-06 16:37:26]

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

[commit-bot: I haz the power, 2017-02-06 16:45:37]

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

[commit-bot: I haz the power, 2017-02-06 16:45:39]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Evan Stade, 2017-02-06 18:06:48]

Nico, could you review ui/vector_icons/DEPS?

[Evan Stade, 2017-02-06 18:06:57]

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

[commit-bot: I haz the power, 2017-02-06 18:07:24]

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

[commit-bot: I haz the power, 2017-02-06 19:09:48]

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

[commit-bot: I haz the power, 2017-02-06 19:09:50]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2017-02-07 17:04:55]

On 2017/02/06 18:06:48, Evan Stade wrote:
> Nico, could you review ui/vector_icons/DEPS?

ping thakis@

[Nico, 2017-02-07 21:33:14]

ui/vector_icons/DEPS lgtm

[Evan Stade, 2017-02-07 21:40:23]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2017-02-07 21:40:23]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2667753006/#ps80001
(title: "fix deps")

[commit-bot: I haz the power, 2017-02-07 21:41:12]

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

[commit-bot: I haz the power, 2017-02-07 23:37:35]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486503623377610,
"parent_rev": "0862c346b88ef3d45157cba19fd14c3aeccbf000", "commit_rev":
"8741905369e3debd9ae7e6eb469fa47a60fbd9b6"}

[commit-bot: I haz the power, 2017-02-07 23:38:12]

Description was changed from

==========
Make new target for vector icon structs to replace
ui/gfx/vector_icons/

Migrate just one icon over for now.

BUG=626786
==========

to

==========
Make new target for vector icon structs to replace
ui/gfx/vector_icons/

Migrate just one icon over for now.

BUG=626786

Review-Url: https://codereview.chromium.org/2667753006
Cr-Commit-Position: refs/heads/master@{#448789}
Committed:
https://chromium.googlesource.com/chromium/src/+/8741905369e3debd9ae7e6eb469f...
==========

[commit-bot: I haz the power, 2017-02-07 23:38:13]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/8741905369e3debd9ae7e6eb469f...