When reviewing another developer's pull request, review comments can fall into a mix of categories. Some changes might be required, while others might be optional. Do you know the best way to tell the requester which is which?
When reviewing pull requests, comments can often be divided into two categories:
By prefixing the comment with one of the categories above, the reviewer can make it clear to the author if they must make the change or not.
When adding 'PBI' comments, a "good Samaritan" approach is to create the PBI and link to it (add a TODO task). Alternatively if you just leave the comment and want the requester to do it, they should do the same (add a TODO with a link to the PBI) before you approve it.
This could be refactored to be better
❌ Figure: Bad example - not clear if the author should make the change or not
Change – check for null reference and throw if found
✅ Figure: Good example - clear that the change should be made before the PR will be approved
Consider – break long method into several smaller ones
✅ Figure: Good example - the reviewer has left a suggestion, but it's up to the author do change or not
PBI – Consolidate these xUnit ‘Fact’ tests to use ‘InlineData’
✅ Figure: Good example - This change doesn't need to be done now, but should be added to the backlog with a comment in the code