Switch nomenclature in about_flags.cc to not use "Experiments".

Switch nomenclature in about_flags.cc to not use "Experiments".

Renames about_flags::Experiment to about_flags::Entry and updates
associated function names and comments.

Also fixes AboutFlagsTest.NoSeparators, which previously only ever
checked the first entry.

CLOSED, superseded by: https://codereview.chromium.org/1407753002/

BUG=490840

[Alexei Svitkine (slow), 2015-05-22 17:14:49]

Patchset #1 (id:1) has been deleted

[Alexei Svitkine (slow), 2015-05-22 17:14:55]

Patchset #1 (id:20001) has been deleted

[Alexei Svitkine (slow), 2015-05-22 18:16:04]

Patchset #1 (id:40001) has been deleted

[Alexei Svitkine (slow), 2015-05-22 18:16:30]

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

[Nico, 2015-05-22 18:20:46]

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
also has "experiments". Also the .js and .css files in the same directory, and
chrome/browser/ui/webui/flags_ui.cc

[Alexei Svitkine (slow), 2015-05-22 18:22:57]

Would you mind if I do those in a follow-up CL?

On Fri, May 22, 2015 at 2:20 PM, <thakis@chromium.org> wrote:

>
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> also has "experiments". Also the .js and .css files in the same directory,
> and
> chrome/browser/ui/webui/flags_ui.cc
>
> https://codereview.chromium.org/1159433002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2015-05-22 18:24:26]

Yes, cause then we end up with two halves of one feature using different
names which is more confusing than what we have now.

On Fri, May 22, 2015 at 11:22 AM, Alexei Svitkine <asvitkine@chromium.org>
wrote:

> Would you mind if I do those in a follow-up CL?
>
> On Fri, May 22, 2015 at 2:20 PM, <thakis@chromium.org> wrote:
>
>>
>>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
>> also has "experiments". Also the .js and .css files in the same
>> directory, and
>> chrome/browser/ui/webui/flags_ui.cc
>>
>> https://codereview.chromium.org/1159433002/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Alexei Svitkine (slow), 2015-05-22 18:29:13]

Ok, will do.

(My personal preference is smaller CLs, but happy to oblige to do it all in
one shot here.)

On Fri, May 22, 2015 at 2:24 PM, Nico Weber <thakis@chromium.org> wrote:

> Yes, cause then we end up with two halves of one feature using different
> names which is more confusing than what we have now.
>
> On Fri, May 22, 2015 at 11:22 AM, Alexei Svitkine <asvitkine@chromium.org>
> wrote:
>
>> Would you mind if I do those in a follow-up CL?
>>
>> On Fri, May 22, 2015 at 2:20 PM, <thakis@chromium.org> wrote:
>>
>>>
>>>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
>>> also has "experiments". Also the .js and .css files in the same
>>> directory, and
>>> chrome/browser/ui/webui/flags_ui.cc
>>>
>>> https://codereview.chromium.org/1159433002/
>>>
>>
>>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Alexei Svitkine (slow), 2015-05-25 17:35:25]

So, looking at the JS/html code closely, it uses the term "experiments" in the
actual UI that's shown to users, e.g.:

"These experimental features may change, break, or disappear at any time."

.. and elsewhere.

For the purpose of this change, my intention was only to change the names in the
source code - since "experiments" to developers generally means "FieldTrials" -
hence to avoid confusion with that.

But for end-users, it's probably fine to use the term "experiments" on the page
- since it's a colloquial term in that context - meaning "things we're trying
out". So I'm leaning towards not changing the UI text.

But this creates a small dilemma - what to do with code that bridges the two.
For example:

  source->AddLocalizedString("flagsNoExperimentsAvailable",
                             IDS_FLAGS_NO_EXPERIMENTS_AVAILABLE);

This refers to the following string:

      <message name="IDS_FLAGS_NO_EXPERIMENTS_AVAILABLE" desc="Shown if no
experiments are available">
        Aww, it looks like there are currently no experiments available.
      </message>

If we're not changing its content, it seems like the current name is appropriate
(i.e. it says "no experiments available" so having a name of
NO_EXPERIMENTS_AVAILABLE makes sense to me.)

So if you agree with the above, what is your suggestion for what should be
changed beyond this CL?

(FWIW, I think changing nomenclature in about_flags.cc is sufficient to reduce
developer confusion with FieldTrials - since developers often edit
about_flags.cc - but rarely does anyone touch flags_ui.cc or the associated
JS/html code.)

[Nico, 2015-05-25 19:43:01]

Can you think of a word that would work in both places? "Features" maybe?

[Alexei Svitkine (slow), 2015-05-26 18:10:36]

Thanks, let me hold off on this change for a bit.

I agree the word "Feature" is good in isolation (and the UI strings could
then change to "experimental feature"), but this actually has a conflict
with a design I have in the works (which is actually what prompted me to
look at this code again and this refactoring). The design aims to introduce
a new Feature API in Chromium, to support a kill-switch mechanism and to
simplify the current somewhat-complicated best practice of using both
FieldTrial and CommandLine APIs.

Design is still in the works, so not yet ready to share widely. So I'll
hold off on this CL for now until I circulate that design and see what the
outcome of that proposal is.

On Mon, May 25, 2015 at 3:43 PM, <thakis@chromium.org> wrote:

> Can you think of a word that would work in both places? "Features" maybe?
>
> https://codereview.chromium.org/1159433002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Alexei Svitkine (slow), 2015-10-09 21:48:03]

So, circling back to this.

At this point, we have base/feature_list.h and corresponding base::Feature. The
next thing on my plate is to add support to about_flags.cc to integrate with it.
(Once that happens, the plan is to send an email to chromium-dev@ to announce
the new API and encourage people to migrate to it from using flag-pairs/field
trials).

Anyway, now that I'm looking at adding the extra support to about_flags.cc, I've
ran against this naming again. Here's the issue:
  - When looking to make my changes, I feel like it's worth to first refactor
and clean up this code a bit.
  - Given the code is already on the plans to be componentized, I figured as
part of the refactoring (mainly splitting up about_flags.cc into helper
classes/files), it would be good to move things to the component.
  - But the code still uses Experiment nomenclature which I still think should
be updated. I don't want to introduce experiment.h to hold the Experiment struct
with the above refactoring.
  - So ideally, I'd like this rename to happen first.

So this brings us back to this rename. I'm not sure renaming Experiment to
Feature is ideal - due to base::Feature now existing. I'm not against still
using the Feature terminology here, since long term it makes sense for most of
the things on about_flags.cc to correspond to base::Feature.

So here are some ideas for names, wdyt?
  1. Experiment -> ExperimentalFeature
  2. Experiment -> FeatureEntry

Downside of 1 is it is a bit more verbose. But still addresses the problem (I
think ExperimentalFeature is much more clearer and harder to confuse with
FieldTrial compared to Experiment). It also allows keeping most of the wording
in chrome://flags UI the same - maybe some updates from "experiments" to
"experimental features", but otherwise ok.

I guess my preference is for 1, despite its verbosity, but I'm ok with 2 or
another suggestion if you prefer. WDYT?

[Nico, 2015-10-09 21:49:36]

They both sound fine to me, so up to you.

[Alexei Svitkine (slow), 2015-10-09 21:49:46]

+droger to CC to give context about the changes to about_flags.cc I'm planning
to make.

[Alexei Svitkine (slow), 2015-10-09 21:51:19]

On 2015/10/09 21:49:46, Alexei Svitkine wrote:
> +droger to CC to give context about the changes to about_flags.cc I'm planning
> to make.

(Mainly see my message in https://codereview.chromium.org/1159433002/#msg13)

> They both sound fine to me, so up to you.

Okay, I'll try to go with ExperimentalFeature and see if the verbosity is
manageable. Will ping the CL again when ready for another look. Thanks!

[Alexei Svitkine (slow), 2015-10-14 19:41:59]

On 2015/10/09 21:51:19, Alexei Svitkine wrote:
> On 2015/10/09 21:49:46, Alexei Svitkine wrote:
> > +droger to CC to give context about the changes to about_flags.cc I'm
planning
> > to make.
> 
> (Mainly see my message in https://codereview.chromium.org/1159433002/#msg13)
> 
> > They both sound fine to me, so up to you.
> 
> Okay, I'll try to go with ExperimentalFeature and see if the verbosity is
> manageable. Will ping the CL again when ready for another look. Thanks!

I went with FeatureEntry in the about_flags code because that made variable
names a bit easier (i.e. with ExperimentalFeature, I'd either had to spell it
all
out for var names, or use "feature" which would be ambiguous with Feature
API integration.)

Sent a new CL with the latest changes:
https://codereview.chromium.org/1407753002/