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

feat(cli): support custom github API hosts #181

Merged
merged 2 commits into from Mar 13, 2020
Merged

Conversation

patrickhulce
Copy link
Collaborator

closes #92

@thedaviddias or anyone else with a github enterprise server would you be able to give this a try? I don't actually have access to a GH enterprise server to test for realz :)

@patrickhulce patrickhulce added the help wanted Extra attention is needed label Jan 14, 2020
@thedaviddias
Copy link

@patrickhulce will try this tomorrow! Thank you!

@patrickhulce
Copy link
Collaborator Author

@thedaviddias any chance you've had time to try this out? :)

@thedaviddias
Copy link

@thedaviddias any chance you've had time to try this out? :)

I'm really sorry, I didn't have yet time to try it. This week has been pretty busy... Will keep you posted when I would have tested out.

@javmeister
Copy link

javmeister commented Mar 9, 2020

Just tested this PR against our github enterprise server.

First run with with my personal token got this output:

GitHub token found, attempting to set status...
No GitHub remote found, skipping.

Looked for the log message and noticed I had to set the env var LHCI_BUILD_CONTEXT__GITHUB_REPO_SLUG manually to our repo url.

The getGitHubRepoSlug() in the runGithubStatusCheck() had returned undefined, I think because our server doesn't have github.com in the url? Haven't debugged that method to confirm this though, but as I said setting the env var override made it work.

Hope this helps, looking forward to have this in my PRs status checks.

@patrickhulce
Copy link
Collaborator Author

Awesome thank you for the info @jalerg! We'll fix that up great catch 👍

@patrickhulce
Copy link
Collaborator Author

aight @jalerg I think we fixed this so you don't need to manually specify the slug, would you be willing to give it one more try? if it doesn't work a sample of your git remote -v output would be amazing :D

@javmeister
Copy link

I didn't have time today to test again, will do as soon as I can tomorrow and get back to you.

@patrickhulce
Copy link
Collaborator Author

Thanks @jalerg! No rush, we really appreciate your help!

Comment on lines 177 to 179
const hash = getCurrentHash();
const slug = getGitHubRepoSlug();
const {githubToken, githubAppToken} = options;
const {githubToken, githubAppToken, githubApiHost} = options;

Choose a reason for hiding this comment

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

The getGitHubRepoSlug() still returned undefined because the githubApiHost parameter wasn't passed in, I still got the No GitHub remote found, skipping. message in the console.

Making this change made it work:

Suggested change
const hash = getCurrentHash();
const slug = getGitHubRepoSlug();
const {githubToken, githubAppToken} = options;
const {githubToken, githubAppToken, githubApiHost} = options;
const {githubToken, githubAppToken, githubApiHost} = options;
const hash = getCurrentHash();
const slug = getGitHubRepoSlug(githubApiHost);

I noticed the urlLabel in the status check message is undefined, probably unrelated to this, but posting here:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

doh! thanks @jalerg! at some point we just need to get a mock github API into our test suite.

Choose a reason for hiding this comment

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

everything worked as expected when making this change, I've run it a few times now, and changed the token to our service account instead of mine and it updated the owner smoothly.

LGTM!

@patrickhulce patrickhulce merged commit 2d2d0b5 into master Mar 13, 2020
@patrickhulce patrickhulce deleted the gh_custom_api_host branch March 13, 2020 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom GitHub status check URL for enterprise setups
3 participants