21

In TFS, when we put comments for things to be fixed in a Pull Request before we will accept it, should we mark it as Reject or Wait For Author? Which is better?

enter image description here

sashoalm
  • 313
  • 1
  • 2
  • 8

1 Answers1

24

As per Microsoft on Review code with pull requests: Vote on changes the suggested purpose of each class of approval is:

Approve with suggestions : Agree with the pull request, but provide optional suggestions to improve the code.

Waiting for the author : Do not approve the changes, and ask the author to review your comments. The author should let you know when you should re-review the code after they have addressed your concerns.

Rejected : The changes aren't acceptable. If you are voting this way, you should leave a comment in the pull request detailing why the changes were rejected.

Thus I take Waiting for Author to mean that the you think the Author screwed up in his/her approach to the solution but that his/her code is redeemable if they take your comments to heart.

And Rejected means that no way in hell are you accepting any change like this no matter how well written the code is.

The question you have to ask yourself is whether your groups idea of the suggested purpose matches Microsoft's idea.

Peter M
  • 2,029
  • 2
  • 17
  • 22
  • 3
    About this: "The author should let you know when you should re-review the code after they have addressed your concerns." - is there a mechanism for the author to alert reviewers that the PR is ready for re-review? – Robert Sim May 16 '18 at 06:01
  • @RobertSim - we use is the 'Ping' option under Reviewers to send a message asking for re-review. – killercowuk Nov 30 '18 at 10:23
  • Another way to look at this: `Waiting for Author` = I want this feature, but you coded it wrong, and I won't approve until you fix it the way I suggest. And `Rejected` = I don't want this feature you are asking me to review. – goku_da_master Apr 22 '20 at 00:05
  • Updated link to MS documentation of vote on changes: https://learn.microsoft.com/en-us/azure/devops/repos/git/review-pull-requests?view=azure-devops&tabs=browser#vote-on-pr-changes – 182764125216 Feb 22 '23 at 16:13
  • @182764125216 Thanks. I've updated it – Peter M Feb 22 '23 at 17:43