Print repo and user context for status commands#278
Print repo and user context for status commands#278vilmibm merged 5 commits intocli:masterfrom vilmibm:pr-status-info
Conversation
|
This is great thank you @vilmibm! ✨ To align more with the context we show for other commands, what I had in mind was something like:
I agree the username feels like overkill here, and we're not showing it anywhere else right now. I actually think we don't necessarily need to show "(fork of x)" since I'd guess you can infer that from the repo name but very open for push back on that! |
|
@ampinsk makes sense! i will retool for that. |
This reverts commit d64dcdc.
|
@ampinsk reverted original version and replaced with simpler thing. body updated with new screenshots. |
|
Yay thank you! Just one more nit: Can we make it all white and include a line break before and after the context line? Basically so it matches the issues/pr list:
|
command/issue.go
Outdated
| out := colorableOut(cmd) | ||
|
|
||
| fmt.Fprint(out, utils.Gray("Relevant issues in ")) | ||
| fmt.Fprint(out, utils.Cyan(ghrepo.FullName(baseRepo))) |
There was a problem hiding this comment.
It's worth noting that if the repository has been renamed, but the user still hasn't updated their git remotes, this will display the pre-rename repo.
If this becomes an issue (i.e. confusing) in the future, we can use something like resolveRemotesToRepos from pr create to find the "resolved" base repo—although, that adds an additional GraphQL query.
There was a problem hiding this comment.
ah, my reverted commits glommed a repo look up onto the status commands in order to determine if the current repo was a fork. i took it out since ampinsk suggested less context was ok but it definitely showed there's only so much confidence we can have about what we're saying without a repo lookup per command. i think it'll be worth the initial performance penalty and once we do it it'll be more clear how best to cache it.
4 participants
Closes #262
This PR adds another line at the top of
gh {pr,issue} statuscommands. It informs the user what repo they're running the command in.It's entirely possible that:
the username is overkilltook this out as per ampinsk's suggestionPlease note that in order to see the "(fork of owner/name)" indicator the parent repo's remote needsnot be named
upstreamorgithub; otherwise, we (intelligently) display status for the parentrepo and report accordingly. This will alert people that they are running status on a fork in the
event their remotes don't line up with what we expect.
I'm not thrilled with how I glommed theparentRepofield into the payloads but I was trying tobalance not making an extra request with not including more in the payloads than needed. I'm open to
suggestion there.
no longer displays whether repo is a fork
All of our other commands print enough context that I didn't think they needed updating.