Help
cancel
Showing results for 
Search instead for 
Did you mean: 
Highlighted
Copilot Lvl 2
Message 1 of 2

Feature Request: group changes, ignore comments

Solved! Go to Solution.

Hi there. 

 

I would have a small feature request that would help us a lot for the productivity in our project. It is about the review feature of pull requests. The feature already improved a lot in the last months with the "Viewed" flagging and some new filter capabilities. 

 

In our project it can happen from time to time that PRs grow a bit bigger. This is not necessarily because the actual change to the product is big. But already a simple refactoring can affect a lot of files to be touched. e.g. let it be a module/namespace rename, or an update of documentation texts along a change. 

 

WIth such kind of changes it easily happens that reviewers are overwhelmed with the number of files touched in a PR. You migth be able to split down certain change into own PRs, but this is not often reasonable from the process and how PRs might need to get merged back as a unit. 

 

To solve this issue I constantly think that the following 2 features on the PR review feature would be amazing: 

 

1. Manual grouping of files changed in a PR. 

Imagine a huge PR with > 100 files changed. maybe 80% of the files were just touched due to a dependency to an actual class you changed in the PR. e.g. you moved 1 class from one namespace/package to another, As a result all your unit tests might get a new using/import. All this changes are just noise and not really important for the reviewer. It would be amazing if the PR opener, has the possibility to move these files into a container/group and label it within the overall PR. In my example above you could split the files into 2 groups: 

 

[Actual Refactoring]

  • src/Namespace/File1.cs -> src/Namespace/Sub/File1.cs

- namespace Namespace {

+ namsepace Namespace.New {

 

[Updated Unit Tests]

  • Modified test/Namespace/File1Tests.cs 
  • Modified test/Namespace/IntegrationTests.cs 

 

This way the reviewer gets the chance to focus on the actual change introduced first and give proper remarks. If the core of the change is clear, it can be checked separately if the other files touched are fine too but maybe the level-of-detail to be checked is way lower. 

 

Also if you have multiple reviewers on one PR, you could split the review into more topics where the responsibles can then focus better. e.g. one part could be the API layer change for a new feature, while the other part is the underlying logic for the change. The API design responsible, can then clearly focus on the API related changes. For this approach it might be needed that you can add a file to multiple groups.

 

Or if we stick to the GitHub approach on organizing things: This feature could also use tags. Each file of a PR can be tagged. And the reviewer can add tag-filters on top before starting the review. 

 

2. Ignore (Single-Line) Comment changes

As of today it is possible to choose if the diff includes whitespaces or not. The next level on that, would be to ignore comment changes. When you try to focus on the code level change, documentation comments might not be that relevant in a first review round of a change. Due to the nature of git-diffs and the complexity of some languages, it might not be possible to provide this feature for multi-line comments. But I think for single-line comments GitHub could hide the related changes. 

 

At least for me these changes would boost productivity a lot. When you work in big teams, with a lot of changes going on, it is easy to loose focus on reviews. Especially the first feature request, would help a lot to stay organized. 

1 Reply
Solution
Moderator
Message 2 of 2

Re: Feature Request: group changes, ignore comments

Hi @Danielku15,

 

Thanks for this feedback! We're always working to improve GitHub and the GitHub Community Forum, and we consider every suggestion we receive. I've logged your feature request in our internal feature request list. Though I can't guarantee anything or share a timeline for this, I can tell you that it's been shared with the appropriate teams for consideration.

 

Cheers!