search

GoogleCloudPlatform / ide-login

The plugins-login-common library for authenticating with the Google Cloud Platform shared by several IDE plugins including Google Cloud Tools for Eclipse, Google Cloud Tools for IntelliJ, and Android Studio.
Apache License 2.0
2 stars 9 forks source link

Return Account for successful sign-in #30

Closed chanseokoh closed 7 years ago

chanseokoh commented 7 years ago

I realized that logIn() should return Account instead of boolean so that a caller knows which Account a user signed in with.

elharo commented 7 years ago

What if instead of returning null on a failed login we threw an exception? Does that make sense? I.e. is a failed login an exceptional condition a client should check for? If we don't do that, they may get an unhandled NullPointerException instead.

chanseokoh commented 7 years ago

It's not always an exceptional case. For example, we allow users to simply log in with the G icon, not doing anything with after login. So I think returning an exception is an overkill.

chanseokoh commented 7 years ago

Forgot to say that users can cancel login in that case.

elharo commented 7 years ago

LGTM assuming tests pass