Clone repos using canonical username/repo name capitalization#2177
Conversation
GitHub user/repo names are case insentitive. This can cause issues when dealing with repos programmatically using systems that aren't aware of this (e.g. go packages). This commit updates the cloning logic of the CLI to query the API for the canonical capitalization and uses that for the clone operation. Fixes: cli#1845
82aff39 to
a5ec03d
Compare
mislav
left a comment
There was a problem hiding this comment.
This is great! Thank you for the contribution 🙇
| cloneDir, err := git.RunClone(cloneURL, opts.GitArgs) | ||
| // If the repo is a fork, add the parent as an upstream | ||
| var parentRepo ghrepo.Interface | ||
| parentRepo, err = api.RepoParent(apiClient, canonicalRepo) |
There was a problem hiding this comment.
Since we're already making an API request in GitHubRepo() above, a potential improvement here could be to expand (or rather, duplicate) that query to also include its parent repository in the response. That way we could get the information about the repo + its parent in a single request.
You are welcome to amend this PR if you're willing to add this tiny optimization, but you can also leave it as-is and our team can get to this as well. 👍
There was a problem hiding this comment.
I've pushed a change that achieves this by loading the parent in the GitHubRepo query - it does mean grabbing a few more bytes than required if you don't care about the parent, but I thought it might be good practice because otherwise the parent (which was already in the model) would be nil - this could cause a bug by implying that the repo isn't a fork (when it is).
GitHub user/repo names are case insentitive. This can cause issues when dealing with
repos programmatically using systems that aren't aware of this (e.g. go packages).
This commit updates the cloning logic of the CLI to query the API for the canonical
capitalization and uses that for the clone operation.
Fixes: #1845