Allow PR reviewers to comment on and suggest changes to unedited lines of code

[dotproto]

One of my favorite changes to GitHub's PR review experience was the introduction of suggested changes. I regularly use this feature in documentation PRs; suggesting changes really helps speed up the review cycle by allowing me to prepare a necessary change and allowing the PR creator to review and accept it rather than me commenting how we should modify the text.

A major drawback to the current implementation, though, is that the GitHub UI does not allow reviewers to suggest changes to lines that are not directly affected by the PR.

Hovering over a line in right next to changed lines gives an 'add line comment' button

Hovering over a line that was hidden behind the "Expand up" or "Expand down" buttons does not display an 'add line comment' button

Please allow reviewers to comment on and suggest changes to any line in the files under review.

July 14, 2023 update: Unmarked @willsmythe's comment as an answer to this request as github/roadmap#347 is still open and on the backlog.

[Arcanorum]

Relevant. isaacs/github#1655

[talesa]

Same from 2014: isaacs/github#284

Seriously, today I've just googled it again thinking to myself "maybe I'm just dumb and I don't know how to do it", just to rediscover all of those threads from 2014, and it turns out my past self went through the same process at some point because I've already upvoted all of them 😅

So here we go again

[davisking]

Yeah, hardly a day goes by that I don't want to comment on some line but the GitHub UI is like "nope, if you want to talk about that line go write out in English author, please look at line 123 and pretend what I'm saying here was written there and stick it on an unrelated but edited line :)

[ChJones-IDM]

How is this not a thing already? GitHub PRs are by far the worst code review tool I've ever used. This would make it a tiny bit better.

[ayyadalaa]

I just needed this feature today, I wanted to point out something that is related to the change but not part of it.

[star-square]

I also needed this feature today in order to comment on some code not changed by the PR but relevant to the PR's changes.

[codesnik]

I'd think that it's a problem to do because code suggestions need to be retained, even as "outdated", even after PR author pushes more commits or even rebases and force-pushes the branch. Probably comments are linked to specific lines in the unified patch of the PR.
But code suggestions outside of the main patch still could work the same, by keeping separate patch layer for suggestions.

[ajilani]

But it does allow you to comment on code that is near the changed code, up to 3 lines away. This seems to me like an arbitrary choice. The functionality appears to already exist for commenting on code that was not changed by the PR. Or are those nearby lines included in the patch file? In that case, I could understand why this is not trivial to implement.

[jdmaguire]

That's my assumption. We're all developers. If something like this isn't already implemented, we can assume there was some structural choice made deep in GH codebase that makes this non-trivial to add in an efficient way.

[Simran-B]

up to 3 lines away. This seems to me like an arbitrary choice

3 lines is exactly the standard amount of context lines in unified diffs. Doesn't seem arbitrary to me.

[TheGiraffe3]

What if you could set a line count somewhere in the .github file?

[whenmoon]

Completely bizarre that this is not possible. Case in point: a PR refactors a file from JS to TS. If a line has not been refactored and therefore changed, how do I point it out to the dev??

[willsmythe]

Thanks for the feedback. We understand the need and are investigating the feasibility this quarter. 2021-12-19 update: Commenting on any line of a file changed in the pull request is on the roadmap for next quarter: github/roadmap#347. Please use #9099 to discuss commenting on unchanged files.

[chriskuech]

Can you at least mark this as unanswered considering the request is objectively not accurately answered?

[muescha]

@chriskuech see "Answer selected by @michellemerrill"

[cyjohnson-ss]

Just started using this, and felt the same need. Thought it would be implemented by now, as this is a couple of year old. OMG

[StingyJack]

Commenting on any line of a file changed in the pull request is on the roadmap for next quarter:

@willsmythe - That was for over a year ago. Is this feature that can prevent defects and improve quality really more important than roadmap items like these (from the roadmap)
github/roadmap#528
github/roadmap#355
github/roadmap#536 - especially this one since its pretty much the same kind of thing

[hyperTwitch]

@willsmythe I see github/roadmap#347 has been closed out. How can we follow along on if/when this will be revived on the roadmap? Looking for something I can follow in order to see when the work gets picked up and when it might be available.

[modernai390]

++

[LittleHuman1]

One of my favorite changes to GitHub's PR review experience was the introduction of suggested changes. I regularly use this feature in documentation PRs; suggesting changes really helps speed up the review cycle by allowing me to prepare a necessary change and allowing the PR creator to review and accept it rather than me commenting how we should modify the text.

A major drawback to the current implementation, though, is that the GitHub UI does not allow reviewers to suggest changes to lines that are not directly affected by the PR.

Hovering over a line in right next to changed lines gives an 'add line comment' button
valid

Hovering over a line that was hidden behind the "Expand up" or "Expand down" buttons does not display an 'add line comment' button
invalid

Please allow reviewers to comment on and suggest changes to any line in the files under review.

Replies
9 suggested answers
·
8 replies

Arcanorum
on Jul 8
Relevant. isaacs/github#1655

image

0 replies
@LittleHuman1 Write a reply

davisking
on Jul 17
Yeah, hardly a day goes by that I don't want to comment on some line but the GitHub UI is like "nope, if you want to talk about that line go write out in English author, please look at line 123 and pretend what I'm saying here was written there and stick it on an unrelated but edited line :)

0 replies
@LittleHuman1 Write a reply

ChJones-IDM
on Sep 8
How is this not a thing already? GitHub PRs are by far the worst code review tool I've ever used. This would make it a tiny bit better.

0 replies
@LittleHuman1 Write a reply

ayyadalaa
on Oct 2
I just needed this feature today, I wanted to point out something that is related to the change but not part of it.

0 replies
@LittleHuman1 Write a reply

star-square
on Oct 2
I also needed this feature today in order to comment on some code not changed by the PR but relevant to the PR's changes.

0 replies
@LittleHuman1 Write a reply

codesnik
on Oct 7
I'd think that it's a problem to do because code suggestions need to be retained, even as "outdated", even after PR author pushes more commits or even rebases and force-pushes the branch. Probably comments are linked to specific lines in the unified patch of the PR.
But code suggestions outside of the main patch still could work the same, by keeping separate patch layer for suggestions.

0 replies
@LittleHuman1 Write a reply

whenmoon
on Oct 13
Completely bizarre that this is not possible. Case in point: a PR refactors a file from JS to TS. If a line has not been refactored and therefore changed, how do I point it out to the dev??

0 replies
@LittleHuman1 Write a reply

willsmythe
on Oct 14
Thanks for the feedback. We understand the need and are investigating the feasibility this quarter. 2021-12-19 update: Commenting on any line of a file changed in the pull request is on the roadmap for next quarter: github/roadmap#347. Please use #9099 to discuss commenting on unchanged files.

8 replies
@CyberFlameGO
CyberFlameGO
on Nov 16
Yeah, that’d mean less people having to make PRs that supersede an already-PR’d change/make PRs to a non—upstream fork (both workarounds are messy) which then has to get merged into the fork (contextually I’m primarily talking about the unmodified files thing, but also the lines thing)

@swythan
swythan
on Nov 25
Another use case that just came up at my company is PR review bots. At the moment if they annotate lines that weren't changed in the PR, then the annotations aren't visible.

Think "Security Linter" where one config change might cause another part of the configuration to become invalid (or to need a warning about the possibly unexpected cascaded change).

@willsmythe
willsmythe
7 days ago
FYI. Commenting on any line of a file changed in the pull request is on the roadmap for next quarter: github/roadmap#347

I created a new discussion specifically about commenting on unchanged files: #9099. Please add any additional comments there (and upvote if this is something you need).

@swythan - can you create a new discussion for your request about review bots and annotations? It wasn't clear if you were asking that unmodified, annotated files just be visible in the Files Changed tab, or for also allowing users to comment on them.

@swythan
swythan
6 days ago
@willsmythe I guess I was assuming that the annotations the bot was making were in some way equivalent to user comments.

I'll forward this to the team that was trying to do this.

@AndrewMonteith
AndrewMonteith
6 days ago
Hi @willsmythe - I've started a discussion for the annotation use case: #9144
Thanks @swythan for altering me to this discussion

@LittleHuman1 Write a reply
Answer selected by michellemerrill

[schollii]

How can we tell the progress of github/roadmap#347, since Q1 is over and it is not possible still, just wondering if there is status somewhere, has it been started or not, almost done, done but waiting for official release, etc.

[inerognes]

Looks like the status is Q4 2022 – Oct-Dec

[db410009]

This is nice!!

But the rational at github/roadmap#347 is also applicable to not modified files
So... progressing what it is nice.
But... the gap (partially) still there

[TWiStErRob]

They moved it to "Future" milestone now, seems like AI has fully taken over, and DX is not as important now :(

[jeffsaremi]

Github is really disappointing compared to Gitlabs. And they're supposed to be the originators.

[StingyJack]

The credited originator of Git has unkind words for GitHub PR's/merges. I have different unkind words about the tooling and flow. That said, it could always be worse (and corrupt history every other week)

[Maher-Reven]

@jeffsaremi don't lose your hope on them man

[tiwaria1]

Upvoted. This would be incredibly useful.

[anurag-bug]

+1

[araujoarthur0]

Please do it, it's weird that Azure DevOps offers such a better reviewing experience.

[fedemengo]

It seems like the best place to push for this is https://github.com/orgs/community/discussions/9099

[dabrahams]

As noted in that discussion's introduction, it's asking for a different feature. Of course, if you had that feature, you wouldn't need this one…

[Andy-INX]

Been using this for ages in Azure DevOps.
Often need to suggest a change to a line the developer didn't touch.
Very useful there, a massive hindrance in Github PRs.
This request seems to have been going since Dec 2021. The lack of attention is mind-boggling.

[marvk]

It's really insane. I liked GitHub in general, but when I started using collaboration features in my new project I was shocked to see how inferior the whole UI and Workflow is to Azure, with PR reviews leading the way. I mean sure, I was cursing Azure at times while using it, but GitHub is a thousand times worse in so many aspects.

[semanser]

I want to share a workaround that may work for someone. While this is far from being ideal and doesn't work for all use cases, it's kind of okay if you're building some kind of automation and all you want is to suggest code changes in another file using the API.

TLDR: It works in reverse of the code-suggesting feature. First, you commit the changes and then give the user the opportunity to automatically revert them.

Here are the steps:

• Instead of suggesting the changes, you need to actually include them in the PR and commit as usual.
• Use the Create a review comment for a pull request API to leave a comment on the specific lines of code (all the "suggested" code from the previous step). Since your changes are already included in the PR, the API works as usual. You can use any content comment body you want, e.g., "Here are some of the changes that I think you want to include. Reply with "decline" to this comment to cancel."
• Listen for a Create a review comment for a pull request Webhook event and look for the event.in_reply_to_id property. Your goal is to set up a keyword that you want to detect; in my case, it's "decline." You can use whatever fits your use case. Whenever you detect the "decline" reply on one of your comments, update (force push?) changes to the PR but now without the "suggested" changes. This leaves the final vote up to the user if they want to keep this change in the PR.

Again, it is very, very far from the ideal solution but it can work for someone who wants to automate code suggestions that have a high chance of being accepted (e.g., automatic code updates).

[Titus-von-Koeller]

That's a neat thought, but still seems kind of hacky. Not sure if it would be really fully functional in the day to day business of maintaining a FOSS project. Would have to be seen in practice.. Are you using this as a day-to-day workflow of yours?

If yes, might be nice to include that in a tool like https://github.com/magit/forge That's by the way how I found your comment. Was searching if the "suggest changes" functionality is covered in the Magit Github integration. At first glance, it seems that it isn't..

[AlexIchenskiy]

Any progress on this? I find GitHub to be a good code-review and collaboration platform, and this feature would be incredibly helpful!

[amrahmed242]

waiting for updates on that :)

[giancarlo-dm]

Sometime we need to point out to a fellow developer where he needs to do the change, and not allowing us to comment on those specific lines because they were not modified is annoying.

[Titus-von-Koeller]

Unfortunate that this is still an issue after almost three years. Seems such an easy fix to activate. Don't see how there is a technical blocker for this. If there is, would be good to verbalize why.

[TWiStErRob]

I think that would be perfectly acceptable, we just need GitHub staff to say this is the reason explicitly. Without that this might or might not be the case, we just don't know ☹. Considering the amount of "any updates?" comments, they have never said this, but if someone has seen an official response, please do correct me with a link!

[dabrahams]

It's far from obvious to me that it requires major changes. You can already comment on the context lines outside the hunk. They could always attach the comment to the nearest hunk (and prioritize earlier hunks if there were two nearest hunks). To be a really high-quality design they might want to adjust the logic for obsoleting comments when PRs are updated, but even if they didn't do that, we'd still be miles ahead of where we are now.

[schollii]

I agree it is likely not so difficult. Github already has to store the comments and all related metadata (date, etc) somewhere that is not in git. Therefore what we're asking here is to add a new type of data, "comment-on-line-not-in-diff", call it whatever, you get the idea. All it would take is 2 things:

• ability to enter a comment on any line of a file that is in the PR's file browser: the data could surely be stored in the same database they currently use for comments, as it would just need to refer to a commit, filename and line number
• ability to create a comment on a file not in the changed files of a PR, so there could be a + somewhere on the file browser view, that would correspond to "add a comment for a file from this repo but not changed yet in this PR". Eg
  • clicking it would allow you to browse the tree of files of the repo to select one of the files, then the file would appear in the list of PR files (possibly a new section at bottom for "unchanged files with comments");
  • you could select the file in the file browser, like any other file there, the only difference is that collapsed view would show "no changes" and if you expanded it, you would see all lines
  • you could click on any line to adda comment, by virtue of the other item in this post

The data to store is again the same, it's just the UI that has to figure out that if a comment is for a file that is not in the changed files list, then it has to go in that section, but even that design is just off-the-cuff, maybe a separate section is not even necessary.

[h-vetinari]

You can already comment on the context lines outside the hunk.

The lines that provide the context to which a hunk is anchored are part of the hunk.

They could always attach the comment to the nearest hunk.

And how would that work exactly? Relative distances change over the evolution of a PR, especially with merges/rebases/etc.

To be a really high-quality design they might want to adjust the logic for obsoleting comments when PRs are updated

With the size of GitHub, all edge cases will be hit almost instantaneously. What you consider really high-quality is IMO the bare minimum for such a feature (figuring out the life cycle of comments over the lifetime of a PR under whatever model you choose for saving comments).

That and they need to ensure that the current hunk-based workflow doesn't break, and also that they're not stuck with an unmaintainable mess of two parallel comment-tracking systems and all the ways these can cross wires.

[dabrahams]

What you consider really high-quality is IMO the bare minimum for such a feature (figuring out the life cycle of comments over the lifetime of a PR under whatever model you choose for saving comments).

Then the existing feature doesn't meet the bare minimum standard.

[ipv6freely]

How on earth is this still not a thing??

[nalbion]

In the GitHub Advanced Security video they're able to create a ````suggestion` with "Outside changed files" targeting line 16 of package.json in GitHub comments. I'd love to know how this is done.

[TWiStErRob]

I would imagine this is (will be?) possible via Sarif fix. It's not (yet?) documented.

Note that it is currently possible to "comment" (put a message on a range of text) on non-changed files via a Sarif upload.

[zzJinux]

For those who are curious where the capture comes from, see this: https://docs.github.com/en/enterprise-cloud@latest/code-security/code-scanning/managing-code-scanning-alerts/triaging-code-scanning-alerts-in-pull-requests#generating-autofix-suggestions-and-publishing-to-a-pull-request and this https://github.blog/2023-11-08-ai-powered-appsec/

[bonnyatomicvaults]

Does anyone at Github read these feature requests? Has Github ever implemented a new requested feature?

[avishekdr]

Dear Github team just remove the if condition that not allowing to add comments on unedited lines. It's creating a pain 😭

[mathstuf]

I doubt it is such a simple thing. I suspect there are database entries for the bits PRs touch; this would mean adding database entries for any file that is commented on during the PR dynamically, not just at PR creation time.

[Linchin]

Any update on this? It would be really helpful.

[alex-rupprecht]

Still waiting for this option

[mgu-voyado]

I too would love to see this feature. My company just switched over to GitHub and I was shocked to see something so fundamental just not be possible, and it hurts my ability to give proper feedback. Please consider it!