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.

Sign up for GitHub

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

Jump to bottom

Submit a suggested change to a PR from the Github CLI #5905

Open
sfullerbeckman opened this issue Jul 8, 2022 Discussed in #5904 · 8 comments
Open

Submit a suggested change to a PR from the Github CLI #5905

sfullerbeckman opened this issue Jul 8, 2022 Discussed in #5904 · 8 comments
Labels
core This issue is not accepting PRs from outside contributors enhancement a request to improve CLI gh-pr relating to the gh pr command

Comments

@sfullerbeckman
Copy link

Discussed in #5904

Originally posted by sfullerbeckman July 8, 2022

You know...

Have you used the "Add a suggestion" button when sumbmitting feedback to a PR? This allows reviewers to submit changes that can be committed directly to the PR branch. This is a great feature and can make it easier for users to collaborate on a PR and saves time, especially if a reviewer already knows how to fix an issue in the PR and can provide a commitable piece of code to the PR.

What if...

But what if we could submit changes to a PR through gh cli? Imagine checking out someone's code with gh pr checkout [pr number]. Then you could look at the person's PR in any IDE or text editor that you wish on your local machine. If you want to submit a suggestion, you would do so directly in your editor. These diff-able changes would be picked up by git of course. But instead of committing those changes directly to the person's PR (which you wouldn't want to do; your code is just a suggestion), you could run a command such as gh pr suggest [pr number]. The cli would pick up staged but uncommited changes and push them directly to the PR as a comment with "suggest changes" that could be directly applied to the PR.

How...

Essentially the process would be...

  1. gh pr checkout [pr number]
  2. Makes some changes to the code in your editor
  3. git add -A
  4. gh pr suggest [pr number]

Why...

Two of the main benefits of a feature like this are as follows:

  1. Someone can do an entire review from their local machine on their own familiar editor: review code, add comments, submit suggestions, etc. No longer would you have to context switch between the browser and their local editor.
  2. By reviewing in the editor, you have all the build tools and static anlysis tools available to you inherantly. The reviewer would now have more confidence in the code he submits to the PR. He can confirm that the suggestion compiles and runs properly.

gh already gives you the ability to review and add comments to PRs. My suggested feature would streamline the proccess of reviewing and submitting suggestions to the PR.
@cliAutomation cliAutomation added the needs-triage needs to be reviewed label Jul 8, 2022
@samcoe samcoe added enhancement a request to improve CLI discuss Feature changes that require discussion primarily among the GitHub CLI team and removed needs-triage needs to be reviewed labels Aug 2, 2022
@vilmibm
Copy link
Contributor

vilmibm commented Aug 24, 2022 •
edited

This idea is very intriguing! After discussing internally we have a few thoughts.

  • As useful as this could be, it's only half of the review process; to leave non-suggestion comments, people would still need to go to the web interface
  • Relying on git is super interesting; we're concerned it could lead to some confusing local state, but it's hard to really think through the UX without a prototype for this. A cleanup step might be necessary

We'd love to see a spike/prototype around this idea and one of us might even pick it up for a hack day, but until it's a more holistic solution to PR reviewing overall we're hesitant to guarantee that we'd merge this kind of feature.

I marked it as core; the community can feel free to explore this but due to the concerns above we don't want to suggest that we'd merge a PR.

@vilmibm vilmibm added core This issue is not accepting PRs from outside contributors and removed discuss Feature changes that require discussion primarily among the GitHub CLI team labels Aug 24, 2022
@mislav
Copy link
Contributor

mislav commented Sep 9, 2022

I'd love to see this, even as an extension to start.

One potential problem that I see with translating local changes to suggested changes is: what would happen with local changes that edit parts of the code that weren't present in the original diff of the PR? As far as I know, one can only suggest changes in areas of the code that the original PR has edited.

@sfullerbeckman
Copy link
Author

sfullerbeckman commented Sep 9, 2022 •
edited

I'd love to see this, even as an extension to start.

One potential problem that I see with translating local changes to suggested changes is: what would happen with local changes that edit parts of the code that weren't present in the original diff of the PR? As far as I know, one can only suggest changes in areas of the code that the original PR has edited.

@mislav, Isn't that going to change soon though? I saw that commenting on lines that weren't changed in the PR is going to be implemented soon. #347

@sfullerbeckman
Copy link
Author

This idea is very intriguing! After discussing internally we have a few thoughts.

  • As useful as this could be, it's only half of the review process; to leave non-suggestion comments, people would still need to go to the web interface
  • Relying on git is super interesting; we're concerned it could lead to some confusing local state, but it's hard to really think through the UX without a prototype for this. A cleanup step might be necessary

We'd love to see a spike/prototype around this idea and one of us might even pick it up for a hack day, but until it's a more holistic solution to PR reviewing overall we're hesitant to guarantee that we'd merge this kind of feature.

I marked it as core; the community can feel free to explore this but due to the concerns above we don't want to suggest that we'd merge a PR.

@vilmibm, addressing your first comment, we could easily add a non-suggestion comment feature to the gh cli. gh pr comment [pr #] [comment text]

And referring to you second comment, it seems to me that whenever the user chooses to submit the suggested changes on his local machine, it would reset the branch's head back to the tip, removing all the suggested changes that were added. This would keep the state clean.

@morrowcj
Copy link

morrowcj commented Dec 2, 2022 •
edited

I'd also love to see this.

Similarly, I'd like to be able to link comments, without suggestions, to specific lines in specific files. Something like gh pr comment [--on-file f] [--lines x | x:y] and add comments to a batch for review: gh pr comment [--batch ["batch name"]]. Both features are present in the browser version.

@rad4day rad4day mentioned this issue Feb 13, 2023
Merged
@sfullerbeckman
Copy link
Author

sfullerbeckman commented Mar 31, 2023 •
edited

Seems like there is a lot of interest for this feature from the discussion post. If anyone has any free time (I do not), I'd love to see a PR mocking out this idea. Maybe if we can demonstrate its feasibility, Github will invest some resources to build it out.

This was referenced Aug 6, 2023
Closed
Closed
@adamslc
Copy link
Contributor

adamslc commented Aug 16, 2023

I would really like to have this functionality to improve the first-time contributor experience. Here is an example workflow:

  1. A new contributor opens a PR,
  2. As part of the automated code-quality checks, a GitHub Action formats the code,
    • Currently, this action probably checks git diff, and errors if the formatter has changed the code (See lint.yml in this repo for instance). I argue that this is unfriendly to new contributors.
    • Using this functionality, I would like to suggest any changes as line-level comments on the PR. There probably needs to be some sort of cutoff that prevents spamming tens of comments for large and poorly formatted PRs.
  3. The contributor directly applies the new formatting to their code, saving them time and frustration.

@yvan-sraka yvan-sraka mentioned this issue Sep 12, 2023
Merged
@andyfeller andyfeller added the gh-pr relating to the gh pr command label Oct 2, 2023
@joshtriplett
Copy link

It looks like non-suggestion comments, as well as reviews, are possible via the CLI now. It would be nice to be able to do the same with suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core This issue is not accepting PRs from outside contributors enhancement a request to improve CLI gh-pr relating to the gh pr command
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add the ability to post suggested changes during reviews adamslc/cli
9 participants
@mislav @vilmibm @joshtriplett @andyfeller @samcoe @adamslc @morrowcj @cliAutomation @sfullerbeckman