8

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

10

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.

  • 1
    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 6: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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.