Skip to content
This repository has been archived by the owner on Sep 9, 2019. It is now read-only.

this address issue #170 #209

Closed

Conversation

ChrisCPO
Copy link

@ChrisCPO ChrisCPO commented Aug 28, 2016

Fixes issue(s) #170.

Changes proposed in this pull request:
/cc relevant people

  • allows a local developer to log-in;
  • bypasses oauth
  • see .sample.env
  • set developer email
  • set skip_team_check to 'true'

* allows a local developer to log-in;
* bypasses oauth
* see .sample.env
* set developer email
* set skip_team_check to 'true'
else
if team_member?
create_user(auth_email)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this pull request, but there should probably be an else here.

@afeld
Copy link
Contributor

afeld commented Aug 28, 2016

This looks like it will be very helpful! Some TODOs:

  • Try without the GitHub credentials set (i.e. avoiding those parts of the setup)
  • Mention how to use this mode in CONTRIBUTING.md
  • Consolidate the two environment variables down to one (e.g. SIGN_IN_AS)

Also, as a convention, it's useful to the project team to make the commit and pull request title something more descriptive, like "added a way to bypass authentication." This makes it easier to understand what the actual change was when looking at the commit history. Thanks again!

@ben-biddington
Copy link

@afeld would you like tests for this stuff?

@ChrisCPO
Copy link
Author

@ben-biddington @afeld Hey will add some tests and edit changes. Virtual box overwrote my desktop packages so had to deal with that on Sunday. Should be able to do this evening.

@jessieay
Copy link
Contributor

jessieay commented Feb 3, 2017

Hi there @ChrisCPO - do you have time to update this?

@jessieay
Copy link
Contributor

Voting to close this without merge since it's over a year stale. Can be re-opened if someone wants to take the time to update it.

@jmhooper
Copy link
Member

jmhooper commented Apr 2, 2018

@jessieay: Agreed

@jmhooper jmhooper closed this Apr 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants