Update --force-fieldtrial-params comment to be a little more clear.

Update --force-fieldtrial-params comment to be a little more clear.

Error out if the param list contains the same fieldtrial,group
multiple times.

BUG=553514
Committed: https://crrev.com/6758a99809fb5a155ad1b113e9e221ef309d0901
Cr-Commit-Position: refs/heads/master@{#374700}

[danduong, 2016-02-10 02:17:20]

danduong@chromium.org changed reviewers:
+ asvitkine@chromium.org, mpearson@chromium.org

[Mark P, 2016-02-10 04:53:42]

lgtm
Thanks for following up on my request.

--mark

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
File chrome/common/variations/variations_util.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
chrome/common/variations/variations_util.cc:61: "Study/Group listed more than
once. (%s.%s)", trial.c_str(),
nit: "Study/group" -> "(Study, group) pair"
This makes more sense as
"Study/Group" could read like "study or group"
and
the (%s.%s) output lends itself to saying "(Study, group)".  Perhaps you want to
use (%s, %s) for even more parallel structure.

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
File chrome/common/variations/variations_util_unittest.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
chrome/common/variations/variations_util_unittest.cc:55:
"AssociateVariationParams.A:a/10/b/test,AssociateVariationParams.A:a/%2F";
As the escape string here isn't needed / important for this test, can you do
something like "x" to make this escape less distracting from the main point?

[Alexei Svitkine (slow), 2016-02-10 17:34:46]

lgtm % comment below & Mark's comments

https://codereview.chromium.org/1682163004/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:594: // changes locally. The argument is param
list of key/value pairs prefixed by an
Nit: The argument is a param list

(Missing "a")

[danduong, 2016-02-10 18:19:53]

https://codereview.chromium.org/1682163004/diff/1/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:594: // changes locally. The argument is param
list of key/value pairs prefixed by an
On 2016/02/10 at 17:34:46, Alexei Svitkine wrote:
> Nit: The argument is a param list
> 
> (Missing "a")

Done.

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
File chrome/common/variations/variations_util.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
chrome/common/variations/variations_util.cc:61: "Study/Group listed more than
once. (%s.%s)", trial.c_str(),
On 2016/02/10 at 04:53:41, Mark P wrote:
> nit: "Study/group" -> "(Study, group) pair"
> This makes more sense as
> "Study/Group" could read like "study or group"
> and
> the (%s.%s) output lends itself to saying "(Study, group)".  Perhaps you want
to use (%s, %s) for even more parallel structure.

Done.

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
File chrome/common/variations/variations_util_unittest.cc (right):

https://codereview.chromium.org/1682163004/diff/1/chrome/common/variations/va...
chrome/common/variations/variations_util_unittest.cc:55:
"AssociateVariationParams.A:a/10/b/test,AssociateVariationParams.A:a/%2F";
On 2016/02/10 at 04:53:41, Mark P wrote:
> As the escape string here isn't needed / important for this test, can you do
something like "x" to make this escape less distracting from the main point?

Done.

[danduong, 2016-02-10 18:24:51]

The CQ bit was checked by danduong@chromium.org

[danduong, 2016-02-10 18:24:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1682163004/#ps20001
(title: " ")

[commit-bot: I haz the power, 2016-02-10 18:26:53]

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

[commit-bot: I haz the power, 2016-02-10 19:28:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 22:30:27]

Description was changed from

==========
Update --force-fieldtrial-params comment to be a little more clear.

Error out if the param list contains the same fieldtrial,group
multiple times.

BUG=553514
==========

to

==========
Update --force-fieldtrial-params comment to be a little more clear.

Error out if the param list contains the same fieldtrial,group
multiple times.

BUG=553514

Committed: https://crrev.com/6758a99809fb5a155ad1b113e9e221ef309d0901
Cr-Commit-Position: refs/heads/master@{#374700}
==========

[commit-bot: I haz the power, 2016-02-16 22:30:27]

Patchset 2 (id:??) landed as
https://crrev.com/6758a99809fb5a155ad1b113e9e221ef309d0901
Cr-Commit-Position: refs/heads/master@{#374700}