-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Conversation
e1bcd6a
to
9ca384e
Compare
packages/firebase_auth/README.md
Outdated
|
||
See the `example` directory for a complete sample app using Firebase Authentication. | ||
First follow the [Google sign-in installation instructions](https://pub.dartlang.org/packages/google_sign_in#-pkg-tab-installing). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change hash: dart-lang/pub-dev@4ba76a4
packages/firebase_auth/README.md
Outdated
|
||
### Import the package | ||
|
||
To use this plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/firebase_auth#-pkg-tab-installing). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change hash: dart-lang/pub-dev@4ba76a4
I've made some modifications, but I don't have a section yet for ios. |
85feb5d
to
e5451aa
Compare
I can say, these instructions would've been really useful to have in the docs. I was really struggling with how to get it working until I went through the Codelab, which covered many of the things. I think these doc changes are quite valuable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I've made a new modification to the ios integration section since. |
packages/firebase_auth/README.md
Outdated
### Import this package | ||
To use this plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/firebase_auth#pub-pkg-tab-installing). | ||
|
||
### Android integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, these steps are required for google sign in to work. Would it make more sense to just document them in the google sign in plugin and link to them from here? That way, we don't have to maintain them in two separate places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally had done that, but then decided being over zealous may be a better choice in making it obvious what to do. So I started copying in the bits. I lean on duplicating in this context b/c there is nothing worse than trying to figuring out all the wiring context when the bits are in two places, at least from my point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine either way, you're call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer having excellent docs on the google sign in plugin and then have a section about "Using firebase auth with google sign in" for this plugin that links to google sign in and makes it clear which steps from there need to be performed to make it work. In the future, there might be additional sections like that for all the other auth mechanisms that firebase_auth supports (but we currently don't support in the flutter plugin).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, when looking at the firebase configuration with the google_sign_in plugin and firebase_auth, it doesn't feel like the same setup that would go on with Android per say. That said. As long as folks have the road laid out, I'm good with that. I can extract the google_sign_in bits and refer to the other plugin then.
packages/firebase_auth/README.md
Outdated
*Note:* When you are debugging on android, use a device or AVD with Google Play services. | ||
Otherwise you will not be able to authenticate. | ||
|
||
### iOS integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
Is this ready to merge? |
I'm finally getting time to work on it again. I'm boiling it down now. |
packages/firebase_auth/README.md
Outdated
|
||
## Getting Started | ||
### Import dependent package | ||
First install the dependency. Follow the [Google sign-in plugin installation instructions](https://pub.dartlang.org/packages/google_sign_in#pub-pkg-tab-installing). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, this is dependent on the pub getting updated. I see the change hasn't been pushed out yet.
c023a95
to
a95d6aa
Compare
Adding the Google sign-in dependency instructions.
Start a fresh review and see what you think. Is this getting closer to what you want? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Adding the Google sign-in dependency instructions.
Adding the Google sign-in dependency instructions.