Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
GitHub Community
Allow PR reviewers to comment on *unchanged* files #9099
Replies: 39 comments 13 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.
-
|
@Twizzle1 that's a perfectly valid use case |
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.
-
|
This is the wrong place to ask this kind of questions. Try to ask at stackoverflow |
Beta Was this translation helpful? Give feedback.
{{title}}
{{editor}}'s edit
{{editor}}'s edit
-
|
The ability to comment not only on unchanged files, but also on unchanged lines of changed files for the very same reason that original poster mentioned. |
Beta Was this translation helpful? Give feedback.
{{title}}
{{editor}}'s edit
{{editor}}'s edit
-
|
+1. My use case is that I'm reviewing a PR where code accessing environment variables was updated to use new references. However some references in other unedited files were not changed; I would ideally like to leave a comment in those files on the relevant lines. |
Beta Was this translation helpful? Give feedback.
-
|
Speaking for everyone who's subscribed to this discussion: Everyone please STOP just commenting If you just want to show your support for this feature request: Please use the If you have a new argument or usecase where this would be relevant: Please double check that it isn't already in a comment. If it is, please just use the Thank you in advance. Footnotes
|
Beta Was this translation helpful? Give feedback.
-
|
If you are the OP, it's easy to find the issue/discussion (just search under Meanwhile, as neither the OP nor a maintainer, I've gotten pinged on the "Organization gists" discussion roughly once every day or two for the last x months, and all I ever find is |
Beta Was this translation helpful? Give feedback.
-
|
How about directing comments about this notification issue to #53075 now that it's got a home? |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
@luke-pk In defense of those complaining, I imagine they are subscribed so that they can be alerted to any progress, discussion, or official acknowledgement of the issue. The core complaint, then, is that they are forced to choose between having an uncluttered inbox and actually staying informed. Unsubscribing doesn't really address that, it's just one of the two evils they aren't satisfied with picking the lesser of. Personally I find all the +1 emails kind of amusing 😛 |
Beta Was this translation helpful? Give feedback.
-
|
Perhaps GitHub needs to provide us a way to contribute this feature back into the mainline. We have enough people on this thread for a full development and QA headcount. |
Beta Was this translation helpful? Give feedback.
-
|
It's a tiny bit awkward to need a PR as the foundation for these comments at all. With the feature as described, if I want to review the state of a repo rather than a change (my typical use case), I can either ask one of the contributors to make a spurious change and file a PR, or I can do it myself… and then my comments will go to… me? It's probably workable but not quite ideal for my use-case. |
Beta Was this translation helpful? Give feedback.
-
|
as a (bad?) workaround you can look in the blame view to see who changed the line of code the last time and add a comment at this commit. |
Beta Was this translation helpful? Give feedback.
-
|
Yes, I've used that bad workaround. Very unsatisfying, I'm afraid. |
Beta Was this translation helpful? Give feedback.
-
|
I still want this :) |
Beta Was this translation helpful? Give feedback.
-
|
Me too:) |
Beta Was this translation helpful? Give feedback.
-
|
Required feature indeed :) |
Beta Was this translation helpful? Give feedback.
-
Hey, July 2023 is almost here - this has taken WAY too long to get implemented. What's up? Is there a targeted release timeframe for this yet? |
Beta Was this translation helpful? Give feedback.
-
|
July is here! And Id like this feature too - google sent me here when looking how to do this |
Beta Was this translation helpful? Give feedback.
-
|
my use case would be an automated "hey you need to bump the version in this config file, here's a suggestion you can commit" what's the latest status here? what's blocking the implementation? |
Beta Was this translation helpful? Give feedback.
-
|
+1 |
Beta Was this translation helpful? Give feedback.
-
|
+1 |
Beta Was this translation helpful? Give feedback.
-
|
I would love this too! Especially for adding translation strings |
Beta Was this translation helpful? Give feedback.
-
|
One can argue on the usage of commenting or referring unchanged code/files but this feature should have been there given the use cases it has for reviewers. |
Beta Was this translation helpful? Give feedback.
-
|
I would love this feature. I think "suggest changes" is a really useful thing and I would love to be able to do it for files not changed in the PR, especially for small changes such as version bumps where the author updated only some of the files but it's a really small change to make. |
Beta Was this translation helpful? Give feedback.
-
|
+1 as well. My use case is requiring an extra test to be added, I would love to comment on the test file even though the PR author did not touch the tests. |
Beta Was this translation helpful? Give feedback.
and others
{{title}}
-
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