Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add more instructions to firebase_auth #76

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

branflake2267
Copy link
Contributor

Adding the Google sign-in dependency instructions.

@branflake2267 branflake2267 force-pushed the patch-6 branch 3 times, most recently from e1bcd6a to 9ca384e Compare May 24, 2017 06:56

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).
Copy link
Contributor Author

Choose a reason for hiding this comment

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


### Import the package

To use this plugin, follow the [plugin installation instructions](https://pub.dartlang.org/packages/firebase_auth#-pkg-tab-installing).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@branflake2267 branflake2267 changed the title Add more instructions Add more instructions to firebase_auth May 25, 2017
@branflake2267
Copy link
Contributor Author

I've made some modifications, but I don't have a section yet for ios.

@branflake2267 branflake2267 force-pushed the patch-6 branch 7 times, most recently from 85feb5d to e5451aa Compare May 25, 2017 04:55
@killermonk
Copy link
Contributor

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.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@branflake2267
Copy link
Contributor Author

I've made a new modification to the ios integration section since.

### 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
Copy link
Member

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.

Copy link
Contributor Author

@branflake2267 branflake2267 May 30, 2017

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.

Copy link
Contributor Author

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.

Copy link
Member

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).

Copy link
Contributor Author

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.

*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
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@collinjackson
Copy link
Contributor

Is this ready to merge?

@branflake2267
Copy link
Contributor Author

I'm finally getting time to work on it again. I'm boiling it down now.


## 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).
Copy link
Contributor Author

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.

@branflake2267 branflake2267 force-pushed the patch-6 branch 2 times, most recently from c023a95 to a95d6aa Compare June 1, 2017 02:51
Adding the Google sign-in dependency instructions.
@branflake2267
Copy link
Contributor Author

Start a fresh review and see what you think. Is this getting closer to what you want?

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@goderbauer goderbauer merged commit e9bf30e into flutter:master Jun 5, 2017
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
Adding the Google sign-in dependency instructions.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants