Meaning of review patch comment ACK and DONE

J. Lewis Muir

unread,

Jul 23, 2019, 12:50:32 PM7/23/19

to Repo and Gerrit Discussion

Hello!

In Gerrit Code Review 2.16.4 (New UI), what is the meaning and intended workflow for the ACK and DONE buttons on a review patch comment?

For example, I added a comment to a line of a patch in a change and then clicked Reply at the top of the change to give a review of +1 (Looks good to me, but someone else must approve).

The author of the change added a comment response, and we agreed that the change is fine but that a new issue should be added to our issue tracking system.

Now Gerrit shows "2 comments (1 unresolved)" for the file, and I navigate to the author's response comment, and it says "Unresolved", and I have the choice of REPLY, QUOTE, ACK, and DONE. What do these mean? I would guess the following:

* REPLY: Reply to the comment.

* QUOTE: Quote part or all of the comment and reply.

* ACK: Acknowledged, and I am not providing an accompanying change.

* DONE: Acknowledged, and I am providing an accompanying change.

I don't even know if those are correct, but if they are, as the reviewer in this case, why would I be providing a change anyway? Wouldn't the author of the change be the one to provide a change?

What should I do; what is the normal workflow here? Would I just click REPLY and give a message of "SGTM" or something? Or would I click ACK to indicate that I've received the author's comment and am happy with the planned resolution. Or would I click DONE to indicate that I consider the issue raised by the comment to be resolved? But then I have a feeling that DONE is something that the author would click, but if that's the case, why is it a choice for me? So, I'm thinking I don't understand the intended meaning and workflow of these choices.

Thank you!

Matthias Sohn

unread,

Jul 23, 2019, 4:27:17 PM7/23/19

to J. Lewis Muir, Repo and Gerrit Discussion

On Tue, Jul 23, 2019 at 6:50 PM J. Lewis Muir <jlm...@imca-cat.org> wrote:

Hello!

In Gerrit Code Review 2.16.4 (New UI), what is the meaning and intended workflow for the ACK and DONE buttons on a review patch comment?

For example, I added a comment to a line of a patch in a change and then clicked Reply at the top of the change to give a review of +1 (Looks good to me, but someone else must approve).

The author of the change added a comment response, and we agreed that the change is fine but that a new issue should be added to our issue tracking system.

Now Gerrit shows "2 comments (1 unresolved)" for the file, and I navigate to the author's response comment, and it says "Unresolved", and I have the choice of REPLY, QUOTE, ACK, and DONE. What do these mean? I would guess the following:

* REPLY: Reply to the comment.
* QUOTE: Quote part or all of the comment and reply.
* ACK: Acknowledged, and I am not providing an accompanying change.
* DONE: Acknowledged, and I am providing an accompanying change.

usually the author replies with DONE if he implemented what you suggested. This helps to keep track, e.g. you may

have given 15 inline comments on larger change and maybe the author doesn't address only 10 of them in the next

patchset, if he replies done on the 10 comments he addressed it's simple for you to spot the remaining 5 comments

which aren't yet addressed.

I don't even know if those are correct, but if they are, as the reviewer in this case, why would I be providing a change anyway? Wouldn't the author of the change be the one to provide a change?

usually the reviewer comments and the author amends the commit in order to address the reviewer's comments

and pushes the amended commit as the next patchset. Sometimes also the reviewer uploads a new patchset

to speed up the review. If in doubt first agree with the author about that, if two developers push new patchsets

for the same change you may end up in a race.

What should I do; what is the normal workflow here? Would I just click REPLY and give a message of "SGTM" or something? Or would I click ACK to indicate that I've received the author's comment and am happy with the planned resolution. Or would I click DONE to indicate that I consider the issue raised by the comment to be resolved? But then I have a feeling that DONE is something that the author would click, but if that's the case, why is it a choice for me? So, I'm thinking I don't understand the intended meaning and workflow of these choices.

I'd say DONE means a previous comment at the same place was addressed, ACK means you agree to what the other

guy said, but no code change is necessary. Overall these responses are a means to help bookkeeping which comments

still need to be addressed.

Thank you!

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/d5b189d9-16c3-40ff-a755-9d2192baf9d8%40googlegroups.com.

J. Lewis Muir

unread,

Jul 23, 2019, 4:47:17 PM7/23/19

to Repo and Gerrit Discussion

On Tuesday, July 23, 2019 at 3:27:17 PM UTC-5, Matthias Sohn wrote:

I'd say DONE means a previous comment at the same place was addressed, ACK means you agree to what the other
guy said, but no code change is necessary. Overall these responses are a means to help bookkeeping which comments
still need to be addressed.

Thank you for your reply! Much appreciated!

On a related note, do ACK and DONE mark the comment in a special way as acknowledged and done, respectively, or are they just shortcuts for clicking REPLY and entering the text "Ack" and "Done", respectively?

David Pursehouse

unread,

Jul 23, 2019, 8:48:42 PM7/23/19

to J. Lewis Muir, Repo and Gerrit Discussion

Clicking those options also sets the "Resolved" flag, which IIUC does not happen if you manually type the text "Ack" or "Done".

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/6213c872-343c-47cc-887d-31c0c83c2965%40googlegroups.com.

Jonathan Nieder

unread,

Jul 23, 2019, 8:50:52 PM7/23/19

to David Pursehouse, J. Lewis Muir, Repo and Gerrit Discussion

David Pursehouse wrote:

On Wed, Jul 24, 2019 at 5:47 AM J. Lewis Muir <jlm...@imca-cat.org> wrote:

On a related note, do ACK and DONE mark the comment in a special way as acknowledged and done, respectively, or are they just shortcuts for clicking REPLY and entering the text "Ack" and "Done", respectively?

Clicking those options also sets the "Resolved" flag, which IIUC does not happen if you manually type the text "Ack" or "Done".

In other words, they are just shortcuts for clicking REPLY, entering the text "Ack" or "Done", and checking "Resolved".

Thanks,

Jonathan

Message has been deleted

J. Lewis Muir

unread,

Jul 24, 2019, 9:50:34 AM7/24/19

to Repo and Gerrit Discussion

Understood! Thanks, all, for your help!

Reply all

Reply to author

Forward