RFC: rename buildflag_header targets (and gni) to buildflag

[Scott Violet]

We currently have two unrelated concepts in Chrome named similarly:

1. A target to generate buildflags. Typically this target is named foo_feature(s) and the corresponding gni named foo_feature.gni. For example, ui/base has ui_features.gni and the target ui_features.

2. base::Feature. This is generally unrelated to (1) and used for features that are user changeable (such as about:flags and/or finch trials). These are typically named with feature in the name as well. For example, ui/base/ui_base_features.h.

Given the two very different meanings I propose we rename the buildflag related targets and files to use buildflags. For ui/base this would mean the following:

. ui_features.gni is renamed to ui_base_buildflags.gni

. rename target ui_features to ui_base_buildflags

Any reason not to do this?

  -Scott

[Ken Rockot]

Seems like an intuitive change to me. 

  -Scott

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKARY_%3D5aofhmLNWgdLLS_gii%3DrkK0M9EnMXV6z9TffN445YgQ%40mail.gmail.com.

[Nico Weber]

Sounds good to me.

There are quite a few buildflag _feature flags. How can we make sure we don't end up converting just a couple, ending up in an even more inconsistent place?

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

[Scott Violet]

My plan is a grep for all gn targets using buildflag_header. Is there a better way to ensure I don't miss anything?

  -Scott

[Alexei Svitkine]

The proposed change sounds good to me too. Thanks for the clean up!

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKARY_%3DFu2fg15AsfjXXDmTdjBX_NPntf4foLTWY%2BGscL_tJBw%40mail.gmail.com.

[Wez]

+1 to this - thanks Scott. :)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKFU2SAsV9zMRf3gwt%3DU3w6jbFAfUOftDhQgd%3DnGzR1i5jeiUg%40mail.gmail.com.

[Dirk Pranke]

I agree that it would be good to better distinguish between compile-time features and runtime/user-changeable features.

However, I wonder a bit how many of the build flags in the *features.gni files aren't actually used in buildflag_header() targets, and why not (or whether that'll cause more confusion).

For example, in ui_features.gni, 6 of the 7 variables are in a declare_args() block, and 6 of the 7 variables are used in //ui/base:ui_features, but those aren't the same 6 (enable_hidpi isn't a declare_arg(), and optimize_webui isn't used in :ui_features).

If everything in these .gni files was in a declare_args() block, I might be tempted to call the files *_build_args instead of buildflags, but if they're not all build args, that might also be confusing.

(I'm not sure I have a better suggestion here, unfortunately).

-- Dirk

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CALekkJe%3Dmy3%3DKGavf4pYDAd7UpSfBKYXko2npPYu%2Bv%2BLXrDzNw%40mail.gmail.com.

[Scott Violet]

I will make sure to address them all. I suppose we could add a PRESUBMIT check to ensure naming consistency, but I'm not really sure that is necessary. WDYT?

On Wed, Feb 21, 2018 at 5:36 AM, Nico Weber <tha...@chromium.org> wrote:

[Nico Weber]

If you convert them all, I'd expect that new ones will end up being consistent with the existing ones, so I'd agree with not needing a presubmit.

[Scott Violet]

Dirk,

I think you're pointing out that we aren't consistent with exactly what lives in *features.gni. This proposal doesn't attempt to solve that (I haven't even realized that until you pointed it out). That said, I think this proposal leaves us in an overall less confusing place then we are currently in. Do you agree?

  -Scott

[Dirk Pranke]

Yes, sorry for being unclear. I think this is a good proposal.

-- Dirk