Credential Manager: JavaScript arguments make more sense in a different order.

Credential Manager: JavaScript arguments make more sense in a different order.

Updated spec, IDL files, and all the *Credential variants.

BUG=400674
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183896

[Mike West, 2014-10-17 12:46:53]

mkwst@chromium.org changed reviewers:
+ vabr@chromium.org

[Mike West, 2014-10-17 12:46:53]

Hey Vaclav, want to take a look at a Blink CL before I find someone Blinky to
review it?

-mike

[vabr (Chromium), 2014-10-17 13:01:31]

LGTM, I don't really have preference for the order, and adding the new
optional parameter for avatar URL looks like a reasonable thing.

Cheers,
Vaclav

On 17 October 2014 14:46, <mkwst@chromium.org> wrote:

> Reviewers: vabr (Chromium),
>
> Message:
> Hey Vaclav, want to take a look at a Blink CL before I find someone Blinky
> to
> review it?
>
> -mike
>
> Description:
> Credential Manager: JavaScript arguments make more sense in a different
> order.
>
> Updated spec, IDL files, and all the *Credential variants.
>
> BUG=400674
>
> Please review this at https://codereview.chromium.org/661973002/
>
> Base URL: https://chromium.googlesource.com/chromium/blink.git/+/master
>
> Affected files (+100, -36 lines):
>   M LayoutTests/http/tests/credentialmanager/credentialscontainer-
> notifyfailedsignin-basics.html
>   M LayoutTests/http/tests/credentialmanager/credentialscontainer-
> notifysignedin-basics.html
>   M LayoutTests/http/tests/credentialmanager/federatedcredential-basics.
> html
>   M LayoutTests/http/tests/credentialmanager/localcredential-basics.html
>   M Source/modules/credentialmanager/FederatedCredential.h
>   M Source/modules/credentialmanager/FederatedCredential.cpp
>   M Source/modules/credentialmanager/FederatedCredential.idl
>   M Source/modules/credentialmanager/LocalCredential.h
>   M Source/modules/credentialmanager/LocalCredential.cpp
>   M Source/modules/credentialmanager/LocalCredential.idl
>   M Source/platform/credentialmanager/PlatformFederatedCredential.h
>   M Source/platform/credentialmanager/PlatformFederatedCredential.cpp
>   M Source/platform/credentialmanager/PlatformLocalCredential.h
>   M Source/platform/credentialmanager/PlatformLocalCredential.cpp
>   M Source/platform/exported/WebFederatedCredential.cpp
>   M Source/platform/exported/WebLocalCredential.cpp
>   M public/platform/WebFederatedCredential.h
>   M public/platform/WebLocalCredential.h
>
>
>

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

[Mike West, 2014-10-17 13:28:52]

mkwst@chromium.org changed reviewers:
+ philipj@opera.com

[Mike West, 2014-10-17 13:28:52]

Philip, I picked your name out of a hat. Would you like to take a look at this
change?

-mike

[philipj_slow, 2014-10-17 13:43:30]

I'm home taking care of my sick son today (but reading email?) so if you want
this reviewed today I recommend drawing another name :) I can do it on Monday
otherwise.

[vabr (Chromium), 2014-10-17 13:47:04]

On 2014/10/17 13:01:31, vabr (Chromium) wrote:
> LGTM, I don't really have preference for the order, and adding the new
> optional parameter for avatar URL looks like a reasonable thing.
I was informed that Rietveld no longer trusts approvals coming via e-mail, so:
LGTM throug Rietveld.
> 
> Cheers,
> Vaclav
> 
> On 17 October 2014 14:46, <mailto:mkwst@chromium.org> wrote:
> 
> > Reviewers: vabr (Chromium),
> >
> > Message:
> > Hey Vaclav, want to take a look at a Blink CL before I find someone Blinky
> > to
> > review it?
> >
> > -mike
> >
> > Description:
> > Credential Manager: JavaScript arguments make more sense in a different
> > order.
> >
> > Updated spec, IDL files, and all the *Credential variants.
> >
> > BUG=400674
> >
> > Please review this at https://codereview.chromium.org/661973002/
> >
> > Base URL: https://chromium.googlesource.com/chromium/blink.git/+/master
> >
> > Affected files (+100, -36 lines):
> >   M LayoutTests/http/tests/credentialmanager/credentialscontainer-
> > notifyfailedsignin-basics.html
> >   M LayoutTests/http/tests/credentialmanager/credentialscontainer-
> > notifysignedin-basics.html
> >   M LayoutTests/http/tests/credentialmanager/federatedcredential-basics.
> > html
> >   M LayoutTests/http/tests/credentialmanager/localcredential-basics.html
> >   M Source/modules/credentialmanager/FederatedCredential.h
> >   M Source/modules/credentialmanager/FederatedCredential.cpp
> >   M Source/modules/credentialmanager/FederatedCredential.idl
> >   M Source/modules/credentialmanager/LocalCredential.h
> >   M Source/modules/credentialmanager/LocalCredential.cpp
> >   M Source/modules/credentialmanager/LocalCredential.idl
> >   M Source/platform/credentialmanager/PlatformFederatedCredential.h
> >   M Source/platform/credentialmanager/PlatformFederatedCredential.cpp
> >   M Source/platform/credentialmanager/PlatformLocalCredential.h
> >   M Source/platform/credentialmanager/PlatformLocalCredential.cpp
> >   M Source/platform/exported/WebFederatedCredential.cpp
> >   M Source/platform/exported/WebLocalCredential.cpp
> >   M public/platform/WebFederatedCredential.h
> >   M public/platform/WebLocalCredential.h
> >
> >
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:blink-reviews+unsubscribe@chromium.org.

[Mike West, 2014-10-17 13:49:45]

On 2014/10/17 13:43:30, philipj wrote:
> I'm home taking care of my sick son today (but reading email?) so if you want
> this reviewed today I recommend drawing another name :) I can do it on Monday
> otherwise.

Ha! I don't need you anymore!

Hope your son gets better soon. Sick kids are no fun. :(

[Mike West, 2014-10-17 13:49:48]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2014-10-17 13:51:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/661973002/1

[commit-bot: I haz the power, 2014-10-17 15:40:05]

Committed patchset #1 (id:1) as 183896