Fix set-default interactive not showing all remotes #6969
Merged
+64
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When running `gh repo set-default` in interactive mode only 5 remotes were showing. This change refactors the methods that fetch the info for those remotes. The methods now expect a limit to be provided. Passing `0` allows running things with no limit. Fixes cli#6854
samcoe
requested changes
Feb 7, 2023
There was a problem hiding this comment.
@despreston This is coming along nicely, thanks for getting started on this work. I added some code suggestions, mostly style and naming ideas.
As for tests, I don't know of any open PRs that are modifying context/context but I would say that I think the only test that is necessary to be added is one in setdefault_test.go confirming that we are resolving more than 5 remotes. What do you think?
samcoe
approved these changes
Feb 7, 2023
There was a problem hiding this comment.
@despreston I went ahead and made the changes I suggested and also wrote up a test. I am planning on getting this into the release later today, hope you don't mind my overstep!
|
@samcoe Not at all, glad I could help. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
When running
gh repo set-defaultin interactive mode only 5 remoteswere showing. This change refactors the methods that fetch the info for
those remotes. The methods now expect a limit to be provided. Passing
0allows running things with no limit.Fixes #6854
Note to reviewers
I was unsure if it made sense to start writing tests for context/context.go. Lots going on in that file and it would suck if someone else has a WIP branch to write those tests.
The description I'm leaving for the
ResolvedRemotes.NetworkReposmethod is questionable and I'd be surprised if someone can't come up with something better.