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

401 Invalid password -> 401 Invalid credentials #86

Closed
wants to merge 3 commits into from
Closed

401 Invalid password -> 401 Invalid credentials #86

wants to merge 3 commits into from

Conversation

breun
Copy link
Contributor

@breun breun commented Feb 21, 2016

Fixed #85

@gr2m
Copy link
Member

gr2m commented Feb 21, 2016

merged via e9e3b0c, thanks @breun. I just squshed the two test: ... commits into one.

As a side node, one thing that helps us with reviews is if you push the test commits first, so that the CI kicks off and we see a failed build for the commit, that then gets green with the fix: or feat: commit, makes sense? No biggie really, just a learning I had myself recently :)

@breun
Copy link
Contributor Author

breun commented Feb 21, 2016

Ah, the instructions said to commit after the test change, but did not mention pushing the commit. I did commit, but only locally. Might be good to make that explicit for other first timers.

Also, the instructions did not mention forking the repository and cloning it from your own account before creating the pull request. That is necessary, right?

@gr2m
Copy link
Member

gr2m commented Feb 21, 2016

ah sorry, I forgot the "push" part, usually we add it. Thanks!

The instructions don’t mention the forking, but link to this tutorial here, which explains everything in detail:
https://egghead.io/series/how-to-contribute-to-an-open-source-project-on-github

Was that your first pull request?

@breun
Copy link
Contributor Author

breun commented Feb 21, 2016

Not really the first, but it's been a while.

@gr2m
Copy link
Member

gr2m commented Feb 21, 2016

it was right on, thanks 👌

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.

None yet

2 participants