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

Explain in docs not to fill out the Enterprise Base URL config setting if the GH Enterprise instance is hosted on GitHub's domain #714

Open
mickmister opened this issue Nov 27, 2023 · 2 comments
Assignees
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Hacktoberfest Help Wanted Community help wanted Tech/Go Type/Enhancement New feature or improvement of existing feature

Comments

@mickmister
Copy link
Member

mickmister commented Nov 27, 2023

We currently support self-hosted GItHub Enterprise servers with custom URLs, configured with the Enterprise Base URL config setting. If the GH Enterprise instance is hosted by GitHub itself, the correct configuration is to leave the Enterprise Base URL config setting blank, so the plugin uses the default api.github.com host for the GitHub client.

We should make it clear in the following places to have this value blank in the case of GitHub hosting the Enterprise instance, and also rename the setting to Custom GitHub Enterprise URL:

  • README.md
  • /github setup flow
  • plugin config settings page

There is also a config setting Enterprise Upload URL, that is always identical to the Enterprise Base URL. I think we should just remove the Enterprise Upload URL since this duplication just causes confusion. We append the suffixes on both of the URLs here

baseURL, _ := url.Parse(config.EnterpriseBaseURL)
baseURL.Path = path.Join(baseURL.Path, "api", "v3")
uploadURL, _ := url.Parse(config.EnterpriseUploadURL)
uploadURL.Path = path.Join(uploadURL.Path, "api", "v3")
client, err := github.NewEnterpriseClient(baseURL.String(), uploadURL.String(), authenticatedClient)

And we are actually setting up the uploadURL path incorrectly. According to the NewEnterpriseClient function we're calling here https://github.com/google/go-github/blob/466e52f0cd17ddbed7a4ac88307fc8fdaf440605/github/github.go#L378:

  • We don't need to apply those suffixes
  • It should be /api/uploads anyway
@mickmister mickmister added Help Wanted Community help wanted Up For Grabs Ready for help from the community. Removed when someone volunteers Hacktoberfest Tech/Go Good First Issue Suitable for first-time contributors Difficulty/1:Easy Easy ticket Type/Enhancement New feature or improvement of existing feature labels Nov 27, 2023
@mickmister mickmister changed the title When using GitHub Enterprise server _hosted by GitHub_, we should explain not to fill out the Enterprise Base URL config setting When using GitHub Enterprise server hosted by GitHub, we should explain not to fill out the Enterprise Base URL config setting Nov 27, 2023
@mickmister mickmister changed the title When using GitHub Enterprise server hosted by GitHub, we should explain not to fill out the Enterprise Base URL config setting Explain in docs not to fill out the Enterprise Base URL config setting if the GH Enterprise instance is hosted by on GitHub's domain Nov 27, 2023
@mickmister mickmister changed the title Explain in docs not to fill out the Enterprise Base URL config setting if the GH Enterprise instance is hosted by on GitHub's domain Explain in docs not to fill out the Enterprise Base URL config setting if the GH Enterprise instance is hosted on GitHub's domain Nov 27, 2023
@LREGS
Copy link

LREGS commented Feb 5, 2024

Can I work on this please?

@mickmister
Copy link
Member Author

Absolutely, thanks @LREGS!

@mickmister mickmister removed the Up For Grabs Ready for help from the community. Removed when someone volunteers label Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Hacktoberfest Help Wanted Community help wanted Tech/Go Type/Enhancement New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

2 participants