Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves error message for $firebaseAuth initialization and updates migration guide #750

Merged
merged 4 commits into from
Jun 2, 2016

Conversation

abeisgoat
Copy link
Contributor

Description

Fixes issues #743 and #749

@jwngr jwngr changed the title Fixes #743 and #749 Improves error message for $firebaseAuth initialization and updates migration guide Jun 2, 2016
@jwngr
Copy link

jwngr commented Jun 2, 2016

@AbeHaskins - I made a commit on top of yours with some cleanup. Please bump googlebot and then merge if you're happy with it. I'll resolve merge conflicts in #748 once this gets merged in.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 94.049% when pulling ae4ef49 on firebaseauth-init-fix into 5463a63 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-83.8%) to 10.044% when pulling a088284 on firebaseauth-init-fix into 5463a63 on master.

| `$authWithCustomToken(token)` | `$signInWithCustomToken(token)` | |
| `$authWithOAuthPopup(provider[, options])` | `$signInWithPopup(provider)` | `options` can be provided by passing a configured `firebase.database.AuthProvider` instead of a `provider` string |
| `$authWithOAuthRedirect(provider[, options])` | `$signInWithRedirect(provider)` | `options` can be provided by passing a configured `firebase.database.AuthProvider` instead of a `provider` string |
| `$authWithOAuthToken(provider, token)` | `$signInWithCredential(credential)` | Tokens must now be transformed into provider specific credentials. This is discussed more in the [Firebase Authentication docs](https://firebase.google.com/docs/auth/#key_functions). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the Firebase Authentication guide.

@katowulf
Copy link
Contributor

katowulf commented Jun 2, 2016

@AbeHaskins a couple nitpicks.

@@ -24,21 +24,36 @@ for details on how to upgrade to the Firebase `3.x.x` SDK.

## `$firebaseAuth` Method Renames / Signature Changes

The `$firebaseAuth` service now accepts an optional Firebase `auth` instance instead of a Firebase
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accepts a firebase.auth.Auth instance instead of a firebase.database.Database instance?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather use natural language here since firebase.auth.Auth doesn't mean much to anyone since the API doesn't look like that. Maybe we can say firebase.auth() instance though?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 94.404% when pulling e0db263 on firebaseauth-init-fix into 5463a63 on master.

@jwngr jwngr assigned katowulf and unassigned abeisgoat and jwngr Jun 2, 2016
@jwngr jwngr merged commit 6adf9cf into master Jun 2, 2016
@jwngr jwngr deleted the firebaseauth-init-fix branch June 2, 2016 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants