commit c4e0b21d294d01f7b805957691384b7a46c6c006
author Edwin Kempin <ekempin@google.com> Thu Apr 28 09:06:03 2022 +0200
committer Edwin Kempin <ekempin@google.com> Thu Apr 28 09:47:57 2022 +0200
tree d06bb5cc681046259ea1f3fba1bfa0de45e31bf0
parent 99b7d2af526275b6ba48e484f4f14fd4ea648cfc

ChangeInserter: Resolve TODO

ChangeInserter contained a TODO about the code block that is adding the
approver as a reviewer, asking if this was still necessary.

Users that have voted on a change are always considered as reviewers,
hence any approver must be contained in the list of reviewers. If a user
creates a new change via push and applies votes at the same type via
push options then this code block is necessary to add the pushing user,
who is an approver by applying votes, as a reviewer. This is covered by
a new assertion in the AbstractPushForReview#pushForMasterWithApprovals()
test.

This means this code block is still necessary and hence we drop the
TODO.

While we are at it rephrase the comment for this code block to make it
clearer that the current user is added as a reviewer because this user
is an approver and approvers always must be reviewers too. Also add
assertions in the test to verify that the pushes were successful.

The same code block also exists in ReplaceOp, which is responsible for
creating new patch sets. Also here when creating a new patch set via
push it's possible to apply votes at the same time via push options.
Also in this case it's necessary to add the approver as a reviewer for
the same reasons as explained above. This is covered by the
AbstractPushForReview#pushForMasterWithApprovals() test and the
AbstractPushForReview#pushNewPatchSetForMasterWithApprovals() test.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I4e4ebc396c5bc0533592ed549c6a654aadb541c4
Release-Notes: skip