We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.I tried to track down the cause of this and think that I've found the problem.It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOp was createdand code was moved out from PostReview. The change that did this is [1]
The comment field in the comment-added stream-event is now "Patch Set 1:\n\n(1 comment)"I'm not sure that this is intended or not, but it makes it impossible to trigger CI checks on comment-added, unless we have missed something. The code that was removed has a todo comment about that is should be removed when patch set level comments are exposed in comment added event./Anders Hanson
--
--
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/77291ce8-e710-4d71-93fb-a11a4476a119n%40googlegroups.com.
On Wed, Nov 23, 2022 at 10:36 PM Anders Hanson <anders....@gmail.com> wrote:We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.I tried to track down the cause of this and think that I've found the problem.It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOp was createdand code was moved out from PostReview. The change that did this is [1]How did you find that this change broke this?
On Thu, Nov 24, 2022 at 2:34 PM Anders Hanson <anders....@gmail.com> wrote:
torsdag 24 november 2022 kl. 09:37:44 UTC+1 skrev eke...@google.com:
On Wed, Nov 23, 2022 at 10:36 PM Anders Hanson <anders....@gmail.com> wrote:
We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)
is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.
I tried to track down the cause of this and think that I've found the problem.It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOp was createdand code was moved out from PostReview. The change that did this is [1]How did you find that this change broke this?
Ouch, I was probably too tired yesterday and missed it.
However, the comment-added stream-event that is sent has the comment field set to comment": "Patch Set 1:\n\n(1 comment)"Even when I add a message in the comment box when doing a reply.
I've looked bit more at the code and it looks like PostReviewOp. insertMessage(ChangeContext ctx) has the code that adds the exacttext that we get in the stream event. It does that in the if msg.isEmpty() block. I'm not that familiar with the Gerrit code base and mightlook at the wrong place.
It looks like the problem is that the in.message is not populated from the draft comments when the UI is does the post review rest call.The content of that call looks like
{"drafts":"PUBLISH_ALL_REVISIONS","labels":{"Code-Review":0,"Gating":0},"ignore_automatic_attention_set_rules":true,"add_to_attention_set":[],"remove_from_attention_set":[],"reviewers":[]}
A draft call was made right before with the following content{"patch_set":1,"message":"recheck","unresolved":false,"path":"/PATCHSET_LEVEL","__unsaved":true}
I've been able to reproduce it , using docker and starting both gerritcodereview/gerrit:3.6.3 and gerritcodereview/gerrit:3.7.0 with all defaults.I'm not much familiar with the publishPatchSetLevelComment option, but its name suggests that it only works for patch set level comments, aka change messages (so what's provided by the message field in CommentInput, which is not set in your example). I don't think it ever worked for inline comments, nor is it supposed to work for them (draft comments are always inline comments).
--
The handling of the publishPatchSetLevelComment option was not lost, but it was just moved into a different class (moved from PostReview into PostReviewOp).
I tried to reproduce this problem from a new test in [2], but it seems this is still working as expected?
The comment field in the comment-added stream-event is now "Patch Set 1:\n\n(1 comment)"
I'm not sure that this is intended or not, but it makes it impossible to trigger CI checks on comment-added, unless we have missed something. The code that was removed has a todo comment about that is should be removed when patch set level comments are exposed in comment added event.
/Anders Hanson
--
--
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/77291ce8-e710-4d71-93fb-a11a4476a119n%40googlegroups.com.
--
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/d2a4ce57-dee9-49c8-970e-2fb510e24c27n%40googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/54cd28c7-ea46-3ae1-fc94-b0b6c2377332%40gmail.com.
On 24 Nov 2022, at 23:58, Luca Milanesio <luca.mi...@gmail.com> wrote:
On 24 Nov 2022, at 21:56, Anders Hanson <anders....@gmail.com> wrote:
On 2022-11-24 22:36, Luca Milanesio wrote:
On 2022-11-24 14:46, Edwin Kempin wrote:
On Thu, Nov 24, 2022 at 2:34 PM Anders Hanson <anders....@gmail.com> wrote:
On Wed, Nov 23, 2022 at 10:36 PM Anders Hanson <anders....@gmail.com> wrote:
We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.I tried to track down the cause of this and think that I've found the problem.
It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOpwas created
torsdag 24 november 2022 kl. 09:37:44 UTC+1 skrev eke...@google.com:On Wed, Nov 23, 2022 at 10:36 PM Anders Hanson <anders....@gmail.com> wrote:We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.I tried to track down the cause of this and think that I've found the problem.It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOp was createdand code was moved out from PostReview. The change that did this is [1]How did you find that this change broke this?Ouch, I was probably too tired yesterday and missed it.However, the comment-added stream-event that is sent has the comment field set to comment": "Patch Set 1:\n\n(1 comment)"Even when I add a message in the comment box when doing a reply.I've looked bit more at the code and it looks like PostReviewOp. insertMessage(ChangeContext ctx) has the code that adds the exacttext that we get in the stream event. It does that in the if msg.isEmpty() block. I'm not that familiar with the Gerrit code base and mightlook at the wrong place.It looks like the problem is that the in.message is not populated from the draft comments when the UI is does the post review rest call.The content of that call looks like{"drafts":"PUBLISH_ALL_REVISIONS","labels":{"Code-Review":0,"Gating":0},"ignore_automatic_attention_set_rules":true,"add_to_attention_set":[],"remove_from_attention_set":[],"reviewers":[]}A draft call was made right before with the following content{"patch_set":1,"message":"recheck","unresolved":false,"path":"/PATCHSET_LEVEL","__unsaved":true}
The handling of the publishPatchSetLevelComment option was not lost, but it was just moved into a different class (moved from PostReview into PostReviewOp).
I tried to reproduce this problem from a new test in [2], but it seems this is still working as expected?
The comment field in the comment-added stream-event is now "Patch Set 1:\n\n(1 comment)"I'm not sure that this is intended or not, but it makes it impossible to trigger CI checks on comment-added, unless we have missed something. The code that was removed has a todo comment about that is should be removed when patch set level comments are exposed in comment added event./Anders Hanson
--
--
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/77291ce8-e710-4d71-93fb-a11a4476a119n%40googlegroups.com.
--
--
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/d2a4ce57-dee9-49c8-970e-2fb510e24c27n%40googlegroups.com.
On 24 Nov 2022, at 13:46, 'Edwin Kempin' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:On Thu, Nov 24, 2022 at 2:34 PM Anders Hanson <anders....@gmail.com> wrote:torsdag 24 november 2022 kl. 09:37:44 UTC+1 skrev eke...@google.com:On Wed, Nov 23, 2022 at 10:36 PM Anders Hanson <anders....@gmail.com> wrote:We upgraded to 3.7.0 last weekend and are now receiving reports that our CI (Zuul-CI)is not triggering builds when people add a comment with the text "recheck".This worked before the upgrade when we were on 3.6.2.I tried to track down the cause of this and think that I've found the problem.It looks like the code that were in PostReview that handled the publishPatchSetLevelComment option was lost when PostReviewOp was createdand code was moved out from PostReview. The change that did this is [1]How did you find that this change broke this?Ouch, I was probably too tired yesterday and missed it.However, the comment-added stream-event that is sent has the comment field set to comment": "Patch Set 1:\n\n(1 comment)"Even when I add a message in the comment box when doing a reply.I've looked bit more at the code and it looks like PostReviewOp. insertMessage(ChangeContext ctx) has the code that adds the exacttext that we get in the stream event. It does that in the if msg.isEmpty() block. I'm not that familiar with the Gerrit code base and mightlook at the wrong place.It looks like the problem is that the in.message is not populated from the draft comments when the UI is does the post review rest call.The content of that call looks like{"drafts":"PUBLISH_ALL_REVISIONS","labels":{"Code-Review":0,"Gating":0},"ignore_automatic_attention_set_rules":true,"add_to_attention_set":[],"remove_from_attention_set":[],"reviewers":[]}A draft call was made right before with the following content{"patch_set":1,"message":"recheck","unresolved":false,"path":"/PATCHSET_LEVEL","__unsaved":true}I'm not much familiar with the publishPatchSetLevelComment option, but its name suggests that it only works for patch set level comments, aka change messages (so what's provided by the message field in CommentInput, which is not set in your example). I don't think it ever worked for inline comments, nor is it supposed to work for them (draft comments are always inline comments).
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CACA_R4LnBATgsH8DFicAuF6mFXrWUoGKpSc8Y_gsUv7K5cWbLQ%40mail.gmail.com.