Open jdeanwallace opened 2 years ago
@jdeanwallace hello and thanks for the feedback! I can definitely see how that's confusing. So in this case you, the author of the PR, made the unresolved comment?
What would have been the most straightforward behavior to you"
I think the awkwardness here comes from the fact that you are the author. If another reviewer had left an unresolved comment, they would be in Request Changes status on GitHub which would have blocked merging (despite the other reviewer Approval)
Hi @samatcodeapprove 👋
So in this case you, the author of the PR, made the unresolved comment?
Yes, that's correct.
What would have been the most straightforward behavior to you?
I'm not too worried about the approval status in CodeApprove, but rather the approval status in GitHub. To me, a review is a team effort requiring 2 or more people to agree on the state of the code. So even if the author ends up reopening a previously resolved thread by marking it an unresolved, I would consider the review incomplete until an agreement is reached.
So yeah I would go for: Withhold the PR approval on GitHub until all comments have been resolved on CodeApprove, regardless of who made them.
I think the awkwardness here comes from the fact that you are the author. If another reviewer had left an unresolved comment, they would be in Request Changes status on GitHub which would have blocked merging (despite the other reviewer Approval)
Yes, this is the behaviour I would expect from CodeApprove seeing as CodeApprove comments only have 2 states "resolved" or "unresolved" which refer to the state of the PR.
Thanks for your consideration 👍
@jdeanwallace thanks for getting back to me! There's definitely a lot to think about here. Mostly things get weird when approval bounces back and forth ... it becomes hard to make sense on both the CA and GH side of things. I am sure there's a big improvement to be had here somewhere I just need to bounce it around in my head a little longer.
The way I understood how CodeApprove works is that once a PR has been approved and all comments have been resolved then CodeApprove automatically makes a final "approved" comment as the reviewer to indicate that the review is complete. For example:
However, if during the review process I resolve all the reviewers comments, but I add an additional unresolved comment, then the final "approved" comment still gets made even though there are still unresolved comments.
Technically the automatic comment is 100% accurate because yes all the reviewers comments have been resolved, but it confused me on what the status of the PR review was in. I expected the final "approved" comment to be made only after all comments (from anyone) have been resolved.