Settings: Users: Provide correct email in API

Settings: Users: Provide correct email in API

We use the email address when removing a user so need to provide
'email' in addtion to the display email.

BUG=722245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2888183002
Cr-Commit-Position: refs/heads/master@{#473673}
Committed: https://chromium.googlesource.com/chromium/src/+/fcd808c7150d3d797b70ace4a7d1231a13301437

[stevenjb, 2017-05-17 15:50:50]

Description was changed from

==========
Settings: Users: Provide correct email in API

We use the email address when removing a user so need to provide
'email' in addtion to the display email.

BUG=722245
==========

to

==========
Settings: Users: Provide correct email in API

We use the email address when removing a user so need to provide
'email' in addtion to the display email.

BUG=722245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[stevenjb, 2017-05-17 15:52:24]

stevenjb@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, tommycli@chromium.org

[stevenjb, 2017-05-17 15:52:25]

rdevlin.cronin@ for minor extensions API change.

[tommycli, 2017-05-17 15:56:12]

lgtm from my perspective thanks

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:14: // Display email for the
user.
nit: Maybe "Displayed email" so it's not confused for a verb. Same with comment
on "Displayed name" too

[Devlin, 2017-05-17 15:56:51]

extensions lgtm

[stevenjb, 2017-05-17 16:05:34]

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:14: // Display email for the
user.
On 2017/05/17 15:56:12, tommycli wrote:
> nit: Maybe "Displayed email" so it's not confused for a verb. Same with
comment
> on "Displayed name" too

Hmm, I feel like "display name" (and equivalent "display email") is the more
common usage (276 results for "display name" in chrome code vs. 8 for "displayed
name"; "display email" is less common but still twice as common as "displayed
email"), despite the potential for confusion (gotta love English). "Displayed"
has it's own problems (it hasn't actually been displayed anywhere yet), and "The
email address to use for display purposes" just sounds silly :)

[stevenjb, 2017-05-17 16:05:37]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-17 16:06:38]

CQ is trying da patch.

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

[Devlin, 2017-05-17 16:16:14]

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/users_private.idl (right):

https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/users_private.idl:14: // Display email for the
user.
On 2017/05/17 16:05:34, stevenjb wrote:
> On 2017/05/17 15:56:12, tommycli wrote:
> > nit: Maybe "Displayed email" so it's not confused for a verb. Same with
> comment
> > on "Displayed name" too
> 
> Hmm, I feel like "display name" (and equivalent "display email") is the more
> common usage (276 results for "display name" in chrome code vs. 8 for
"displayed
> name"; "display email" is less common but still twice as common as "displayed
> email"), despite the potential for confusion (gotta love English). "Displayed"
> has it's own problems (it hasn't actually been displayed anywhere yet), and
"The
> email address to use for display purposes" just sounds silly :)

If we *really* wanted to bikeshed this, "emailForDisplay" or "nameForDisplay" is
probably most accurate.  But I agree with Steven that, in this case, consistency
is probably more useful.  Maybe we can keep the variables the same and just
update these comments, "Display-friendly email for the user" or something
similar?

[tommycli, 2017-05-17 16:17:23]

On 2017/05/17 16:16:14, Devlin (catching up) wrote:
>
https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
> File chrome/common/extensions/api/users_private.idl (right):
> 
>
https://codereview.chromium.org/2888183002/diff/1/chrome/common/extensions/ap...
> chrome/common/extensions/api/users_private.idl:14: // Display email for the
> user.
> On 2017/05/17 16:05:34, stevenjb wrote:
> > On 2017/05/17 15:56:12, tommycli wrote:
> > > nit: Maybe "Displayed email" so it's not confused for a verb. Same with
> > comment
> > > on "Displayed name" too
> > 
> > Hmm, I feel like "display name" (and equivalent "display email") is the more
> > common usage (276 results for "display name" in chrome code vs. 8 for
> "displayed
> > name"; "display email" is less common but still twice as common as
"displayed
> > email"), despite the potential for confusion (gotta love English).
"Displayed"
> > has it's own problems (it hasn't actually been displayed anywhere yet), and
> "The
> > email address to use for display purposes" just sounds silly :)
> 
> If we *really* wanted to bikeshed this, "emailForDisplay" or "nameForDisplay"
is
> probably most accurate.  But I agree with Steven that, in this case,
consistency
> is probably more useful.  Maybe we can keep the variables the same and just
> update these comments, "Display-friendly email for the user" or something
> similar?

"Display-friendly foo" is awesome. Not bikeshed worthy... but I will keep the
"display-friendly foo" idea in my head... that's probably the least confusing
one.

[commit-bot: I haz the power, 2017-05-17 17:46:10]

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

[commit-bot: I haz the power, 2017-05-17 17:46:11]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[stevenjb, 2017-05-22 18:37:34]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-22 18:38:23]

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-05-22 20:12:25]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495478254611380, "parent_rev":
"a9244202f6597e42e2dc4ffb1a410114f5a16a9b", "commit_rev":
"fcd808c7150d3d797b70ace4a7d1231a13301437"}

[commit-bot: I haz the power, 2017-05-22 20:12:32]

Description was changed from

==========
Settings: Users: Provide correct email in API

We use the email address when removing a user so need to provide
'email' in addtion to the display email.

BUG=722245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Settings: Users: Provide correct email in API

We use the email address when removing a user so need to provide
'email' in addtion to the display email.

BUG=722245
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2888183002
Cr-Commit-Position: refs/heads/master@{#473673}
Committed:
https://chromium.googlesource.com/chromium/src/+/fcd808c7150d3d797b70ace4a7d1...
==========

[commit-bot: I haz the power, 2017-05-22 20:12:34]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/fcd808c7150d3d797b70ace4a7d1...