Allow PR reviewers to comment on *unchanged* files #9099
Replies: 105 comments 21 replies
-
I have a use case in hand that could be solved by this proposed change. I have a PR that makes some small changes in a specific area of the codebase. This whole area was added recently, but for unrelated reasons, I've not had an opportunity to review this whole area at all when it was added. And since the current PR has drawn my attention to this - I want to use this opportunity, expand the scope of PR and review the whole newly added area altogether, including files that haven't been changed by this specific PR in hand. This is similar to a code audit, i.e. situation where a review is needed on a codebase in it's present state, without any changes whatsoever, but I guess it would be much easier to allow commenting on any files within a context of PR than to invent a whole new concept of a code audit outside of PR context. |
Beta Was this translation helpful? Give feedback.
-
Yea this is a much needed feature. Some codebases can be complex and good reviews might quickly grow in scope from just the changes of the dev to a much larger set if the PR being reviewed touches/interacts with many systems indirectly |
Beta Was this translation helpful? Give feedback.
-
I gree this would be very useful as well, has there been any update on this proposed feature? |
Beta Was this translation helpful? Give feedback.
-
Extremely necessary for translators – when you need to correct mistakes in an already existing translation file, but missed by the author of an already opened PR. |
Beta Was this translation helpful? Give feedback.
-
I have another use case for this. A PR contained changes that caused stale code in another file. I couldn't add a comment on the stale code in my PR review. |
Beta Was this translation helpful? Give feedback.
-
I would also like to have this feature. I frequently find myself needing to do this. |
Beta Was this translation helpful? Give feedback.
-
This would be very useful for large codebases in which old code needed to be removed/changed but was not... having to reference a line # and/or link to the code in a comment is very inconvenient. |
Beta Was this translation helpful? Give feedback.
-
Agreed that this would be critically useful. It would be great to see this happen! |
Beta Was this translation helpful? Give feedback.
-
Adding my support to this request, for exactly the reason brought up by @jordanbtucker |
Beta Was this translation helpful? Give feedback.
-
Really surprised this doesn't exist tbh - I thought I simply didn't know how to find it. But yes, it's extremely common during a PR review to find that a dev missed some related changes elsewhere in the codebase, & it seems currently there's no way to flag those. i.e. maybe they're refactoring references to a particular entity, but missed one of those references. As far as I can tell, there's no way to propose that as a change, in the spot where it actually needs to happen. |
Beta Was this translation helpful? Give feedback.
-
Surprised that this feature does not exist in the most widely used collaboration tool in 2022. Would love to have it, could help a lot to improve the PR review experiences. |
Beta Was this translation helpful? Give feedback.
-
Wanted to add a code suggestion that needed certain configuration in a file that wasn't changed. Felt unnatural to not be able to add this during the review but in another comment. |
Beta Was this translation helpful? Give feedback.
-
+1 In a PR I want to indicate that some code should be deleted, but since it wasn't touched, there's no way to do so. |
Beta Was this translation helpful? Give feedback.
This comment was marked as off-topic.
This comment was marked as off-topic.
-
We're in 2024 and we still need this. |
Beta Was this translation helpful? Give feedback.
-
Yes , we need this. |
Beta Was this translation helpful? Give feedback.
-
Just did a code review and wanted to add to the review that my colleague also should change line x to y, because z. Then I noticed... Thats not possible?! |
Beta Was this translation helpful? Give feedback.
-
Is there any progress about this feature? |
Beta Was this translation helpful? Give feedback.
-
Yes, really useful and necessary. |
Beta Was this translation helpful? Give feedback.
-
Considering they just put Commenting on unchanged lines in a pull request into the wood chipper, I don't think this will get done. |
Beta Was this translation helpful? Give feedback.
-
This would be really useful, to point out directly where extra changes would be needed to address a PR's intended changes |
Beta Was this translation helpful? Give feedback.
-
I would love this feature as well. |
Beta Was this translation helpful? Give feedback.
-
+1 for this feature. Sometimes context is important, for example when pointing out precedence for some kind of a design or code style choice in a different file, or if there is a similar problem that was solved in an established way, and the PR deviates from it. |
Beta Was this translation helpful? Give feedback.
-
Another use case of this feature would be for code generator validation. It's a very common mistake for folks to change a source file and then forget to re-generate the outputs and commit them too. Or the reverse and edit a generated file. I would very much like to be able to re-generate the code in CI and suggest a change if there's a diff. |
Beta Was this translation helpful? Give feedback.
-
I'd really like to see this feature implemented. Often when I'm reviewing code, a change that I suggest will require changes elsewhere in the unaffected portion of the code. It would be good to be able to add review comments there so I can point out what those changes might be. |
Beta Was this translation helpful? Give feedback.
-
This feature is absolutely required! |
Beta Was this translation helpful? Give feedback.
-
Kinda hilarious that gitlab has had this issue open for 6 years and also not addressing it -- https://gitlab.com/gitlab-org/gitlab/-/issues/24328 I was hoping to use 'GitLab does it" to motivate the folks at GH but I guess not! |
Beta Was this translation helpful? Give feedback.
-
I've been harping on NI for their diff tooling not providing enough context when I'm reviewing code I haven't looked at before and need to review more scope than just the code changes.... Looks like GH doesn't support this capability either... (Looking at you, closed issue for being able to comment on unchanged lines that was in the roadmap) I'm currently reviewing some stuff that was reviewed by others previously and in so needed to pull down additional files and get better context on the smaller changes that were made. In doing so I noticed other fixes that needed to be made but I can't comment on it in the PR! As an new lead developer on a team reviewing team members' changes, I need easier access to more scope around the changes to know the changes themselves are correct and the ability to comment elsewhere in files that were changed and other files. |
Beta Was this translation helpful? Give feedback.
-
Allow PR review comments on files not included (unchanged) in a pull request. This is useful when a PR reviewer needs to tell the PR author about a change they may have missed in a file that was not changed in that pull request.
From @mabwah-arista
This discussion was split out from an existing, related discussion 4452 that focused on commenting on any line of a changed file.
Beta Was this translation helpful? Give feedback.
All reactions