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

Fix enterprise github auth #74

Merged
merged 2 commits into from
Aug 30, 2018

Conversation

tjaneczko
Copy link
Collaborator

We need to pass the authUrl to ghauth when working with GitHub enterprise, otherwise it'll try to use a token from github.com

@tjaneczko
Copy link
Collaborator Author

tjaneczko commented Aug 27, 2018

@ungoldman Fixed up GitHub enterprise auth and url detection and tested it on our GitHub enterprise server. Everything works with these changes! Could you review whenever you get a chance? Would be awesome to get this in

@ungoldman
Copy link
Owner

Looks good at first glance, will try to test it out in the next 24 hours!

@tjaneczko
Copy link
Collaborator Author

Awesome, thanks! Also, thanks for gh-release in the first place, it's an awesome tool that really helps to streamline our workflow

@tjaneczko
Copy link
Collaborator Author

@ungoldman Hate to bug you about this, but any chance you might be able to take another look at this PR sometime today or tomorrow? Also wondering if you guys are looking for help maintaining this project, or with any of the other issues that are open on it. I was considering tackling #49 when I get some free time as that would be a really nice enhancement to gh-release 😄 Thanks!

@ungoldman ungoldman merged commit 3486bb4 into ungoldman:master Aug 30, 2018
@ungoldman
Copy link
Owner

ungoldman commented Aug 30, 2018

LGTM 👍

Also yes, I would be happy to welcome another maintainer and please do feel free to tackle any issues!

All we ask is you read the collaborator guidelines: https://github.com/hypermodules/gh-release/blob/master/CONTRIBUTING.md#rules

Generally any change should be in a PR, and should be approved by a maintainer who didn't open the PR before being merged.

Also this module is used by a fair number of projects so the main thing is to try to be careful not to introduce breaking changes lightly, and to try to be backwards compatible with current functionality whenever possible.

Thank you @tjaneczko! (invite sent)

@ungoldman
Copy link
Owner

📦 3.2.3

@jgravois
Copy link
Collaborator

fwiw, i tested this branch in GitHub enterprise and everything was 👍

@ungoldman
Copy link
Owner

Thanks @jgravois 🙏

@tjaneczko tjaneczko deleted the fix-enterprise-auth branch August 30, 2018 17:28
@tjaneczko
Copy link
Collaborator Author

Awesome, thanks for reviewing @ungoldman and thanks for testing @jgravois !! Accepted the invite, the collaborator guidelines all look kosher and I'm happy to help out with contributing and maintaining this project!

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