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

Add URL config for GitHub Enterprise #101

Merged

Conversation

Cellivar
Copy link
Contributor

@Cellivar Cellivar commented Apr 8, 2019

I think I ran the pester tests correctly, or at least they all came back green for me anyhow. Static analysis also came back empty.

This resolves #98 with the ability to supply a new configuration parameter, GitHubBaseUrl. It will default to github.com. GitHub Enterprise installs to a different URL and the API remains as api.somegithub.com, thus it's simple to hit that API on a different base URL.

I've been using a similar set of changes along with a few other quality of life ones in my organization over the past week as we prototyped out some tools around automating GitHub Enterprise. It performed admirably with these changes here.

Fixed a related typo in the documentation as well.

@msftclas
Copy link

msftclas commented Apr 8, 2019

CLA assistant check
All CLA requirements met.

@Cellivar
Copy link
Contributor Author

Cellivar commented Apr 8, 2019

AppVeyor CI is not visible for me so I can't check :(

Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Thanks so much for this feature. It sounds like this should definitely further open up the available userbase.

I've included some minor change requests before this would be merged in.

I think it would also be great to include a minor documentation update to point out this specific configuration. It probably makes sense to call it out within the Configuration section of the README.

Thanks again!

CONTRIBUTING.md Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubCore.ps1 Outdated Show resolved Hide resolved
GitHubCore.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
GitHubConfiguration.ps1 Outdated Show resolved Hide resolved
Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Update looks great. I like what you've done here. SUPER minor additional feedback, and then we're ready to pull this in and release a module update.

Thanks again!

GitHubCore.ps1 Outdated Show resolved Hide resolved
GitHubCore.ps1 Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
Copy link
Member

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

This is a great update. Thanks so much for your contribution!

@Cellivar
Copy link
Contributor Author

No problem! Thank you for your patient guidance while I shook the rust off my public contribution skills.

@HowardWolosky HowardWolosky merged commit d5acd0f into microsoft:master Apr 11, 2019
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.

Support GitHub Enterprise
3 participants