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

Support setting GitHub URLs via configuration #330

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Support setting GitHub URLs via configuration #330

merged 1 commit into from
Aug 7, 2024

Conversation

abhinav
Copy link
Owner

@abhinav abhinav commented Aug 7, 2024

Setting GITHUB_URL and GITHUB_API_URL environment variables
can be inconvenient.
Allow setting these values via git config if desired.

@abhinav
Copy link
Owner Author

abhinav commented Aug 7, 2024

This change is part of the following stack:

Change managed by git-spice.

Base automatically changed from doc-config-keys to main August 7, 2024 14:56
Setting GITHUB_URL and GITHUB_API_URL environment variables
can be inconvenient.
Allow setting these values via git config if desired.
@abhinav abhinav enabled auto-merge (squash) August 7, 2024 14:57
@abhinav abhinav merged commit 23351a7 into main Aug 7, 2024
7 of 10 checks passed
@abhinav abhinav deleted the github-conf branch August 7, 2024 14:58
@messense
Copy link

messense commented Aug 8, 2024

It'd be really nice if git-spice can infer GHE server and API urls from git remote URL in .git/config when it's not github.com.

@abhinav
Copy link
Owner Author

abhinav commented Aug 8, 2024

It'd be really nice if git-spice can infer GHE server and API urls from git remote URL in .git/config when it's not github.com.

I agree, but unfortunately, unless there's a clear way to identify GHE servers (e.g. they must always have github.com in their name), that's not possible.
While git-spice currently only supports GitHub, I've intentionally tried to keep it forge-agonistic.
GitHub-specific details are in a single place. Everything outside that assumes that a repository could be using any forge.
It is possible and conceivable to add support for other code forges to git-spice.
In fact, most of the CLI behavior tests run against a fake forge with a simpler API that is registered only for tests.

@messense
Copy link

messense commented Aug 9, 2024

Maybe there should be an option to configure the git server type? Then you can infer the API urls based on server type and remote URL, it's just nicer to not have to explicitly write out multiple URLs.

@abhinav
Copy link
Owner Author

abhinav commented Aug 9, 2024

Maybe there should be an option to configure the git server type? Then you can infer the API urls based on server type and remote URL, it's just nicer to not have to explicitly write out multiple URLs.

Oh, hm. 🤔 Are you suggesting a way to say, "assume all remotes are GitHub"?
I can see that being more convenient than the version in this PR.

@abhinav
Copy link
Owner Author

abhinav commented Aug 9, 2024

Oh, I think I understand what you mean.
The configuration would just state that this is a GHES server, and (I guess?) the API URL for those is always /api on the instance URL?

I ask because I agree that that's a better UX, but I'm trying to determine whether that's mutually exclusive with the configuration option offered here. I'll revert this PR if that's the case.
If they're not mutually exclusive, I'll leave this PR in, and that option can happen separately.

@messense
Copy link

messense commented Aug 9, 2024

The configuration would just state that this is a GHES server, and (I guess?) the API URL for those is always /api on the instance URL?

exactly, I don't think they are mutually exclusive, it's just a shortcut IMO.

@abhinav
Copy link
Owner Author

abhinav commented Aug 9, 2024

Created #336.

Separately, @messense, re this:

it's just nicer to not have to explicitly write out multiple URLs.

One obvious convenience we can add here: if GitHub URL is not github.com, then API URL is GitHub URL + "/api". That would help here a little, yes?

@messense
Copy link

messense commented Aug 9, 2024

One obvious convenience we can add here: if GitHub URL is not github.com, then API URL is GitHub URL + "/api". That would help here a little, yes?

Yes, I think so.

abhinav added a commit that referenced this pull request Aug 9, 2024
For GHES, usually the API URL is /api under the base URL.
So if the API URL is not set, and base URL is not github.com,
we can assume the API URL is $baseURL/api.

Users can still override this with the `spice.forge.github.apiUrl`
or `GITHUB_API_URL` environment variable.

Follow up to #330 (comment)
abhinav added a commit that referenced this pull request Aug 9, 2024
For GHES, usually the API URL is /api under the base URL.
So if the API URL is not set, and base URL is not github.com,
we can assume the API URL is $baseURL/api.

Users can still override this with the `spice.forge.github.apiUrl`
or `GITHUB_API_URL` environment variable.

Follow up to
#330 (comment)
Relates to #336
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.

2 participants