Skip to content

Conversation

@puilp0502
Copy link
Contributor

@puilp0502 puilp0502 commented Mar 6, 2017

Addresses #623.

I didn't know if I should bump up the version, so I left it as it is.

@samtstern
Copy link
Contributor

@puilp0502 just wanted to say I know this is lingering, I have been caught up on some other stuff and will test this soon. Thank you for doing it!

@samtstern samtstern changed the base branch from master to version-2.0.0-dev March 9, 2017 19:26
@samtstern samtstern changed the base branch from version-2.0.0-dev to master March 9, 2017 19:27
@samtstern samtstern changed the base branch from master to version-2.0.0-dev March 9, 2017 19:28
@samtstern
Copy link
Contributor

@puilp0502 FYI I changed the target branch to version-2.0.0-dev as that's where the next version will come from. You may have to change your local setup a bit, let me know if that's confusing.

@samtstern
Copy link
Contributor

samtstern commented Mar 9, 2017

Ok I just went and got a new phone number (thanks Google Voice) and then made a Facebook account that has no email. Then I signed in and got the following (see below).

I am surprised that signing in like that even works! I guess email is not a hard requirement for Facebook accounts (learn something new about your own products every day haha).

I am fine to merge this change, but @amandle can you chime in and let me know if this could cause any potential problems?

Console:
screenshot from 2017-03-09 12 02 03

App:
no_email

@amandle
Copy link
Contributor

amandle commented Mar 15, 2017

LGTM, Twitter also doesn't require email be shared, so I don't foresee this change causing problems

@samtstern samtstern merged commit 1b1cdb5 into firebase:version-2.0.0-dev Mar 15, 2017
@samtstern samtstern added this to the 2.0.0 milestone Mar 15, 2017
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.

3 participants