Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-40059

Gerrit Trigger CommentAdded running on every comment due to stream event changes.

    Details

    • Similar Issues:

      Description

      With the new changes added in https://gerrit-review.googlesource.com/#/c/71051 a couple of patches were put through to update this functionality.

      https://github.com/jenkinsci/gerrit-trigger-plugin/commit/5d531e6631fad490dd577fd083ba442ff23a0d95

      This logic appears to be incorrect as the initial patch/functionality that there would always be a present boolean json param of 'updated' which was always present that would track the state change.
      In a subsequent patch they put in optionally when the value had changed "oldValue", whilst the committer kept the same logic for this, but as the oldValue on new versions of gerrit 2.13 and later, this doesn't work as it will still fall back to the old method of checking whether to run a job on commentAdded.

      Some sort of version checking appears to be the option if backwards compatibility is desired (which the comment states that it is).

        Attachments

          Activity

          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Tyrone Abdy
          Path:
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/version/GerritVersionChecker.java
          http://jenkins-ci.org/commit/gerrit-trigger-plugin/0a8e1c246c1b3bb08733a5ef9345c8f13023a328
          Log:
          JENKINS-40059 CommentAdded trigger firing on every comment

          The CommentAdded trigger was firing on any comment after
          the necessary value of an approval had been met.
          This was happening due to that in previous versions of gerrit
          before 2.13.0 approval information only appeared once
          something had changed.
          This changed to use the oldValue that would optionally
          be included on state change. The logic added
          to the gerrit-trigger-plugin was based on an earlier
          patchset that had an ever present variable for state
          change.
          This led to a fall through to the old style of checking
          for whether to trigger based on a comment.

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Tyrone Abdy Path: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/version/GerritVersionChecker.java http://jenkins-ci.org/commit/gerrit-trigger-plugin/0a8e1c246c1b3bb08733a5ef9345c8f13023a328 Log: JENKINS-40059 CommentAdded trigger firing on every comment The CommentAdded trigger was firing on any comment after the necessary value of an approval had been met. This was happening due to that in previous versions of gerrit before 2.13.0 approval information only appeared once something had changed. This changed to use the oldValue that would optionally be included on state change. The logic added to the gerrit-trigger-plugin was based on an earlier patchset that had an ever present variable for state change. This led to a fall through to the old style of checking for whether to trigger based on a comment.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Robert Sandell
          Path:
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/version/GerritVersionChecker.java
          http://jenkins-ci.org/commit/gerrit-trigger-plugin/a36413b2b88ec89e0e525c08bade676f74e34048
          Log:
          Merge pull request #302 from tyroneabdy/master

          JENKINS-40059 CommentAdded trigger firing on every comment

          Compare: https://github.com/jenkinsci/gerrit-trigger-plugin/compare/b0b2c86fb3c9...a36413b2b88e

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Robert Sandell Path: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/version/GerritVersionChecker.java http://jenkins-ci.org/commit/gerrit-trigger-plugin/a36413b2b88ec89e0e525c08bade676f74e34048 Log: Merge pull request #302 from tyroneabdy/master JENKINS-40059 CommentAdded trigger firing on every comment Compare: https://github.com/jenkinsci/gerrit-trigger-plugin/compare/b0b2c86fb3c9...a36413b2b88e
          Hide
          romanz Roman Zwi added a comment -

          Is Status:OPEN still correct?

          Show
          romanz Roman Zwi added a comment - Is Status:OPEN still correct?
          Hide
          tabdy Tyrone Abdy added a comment -

          https://github.com/jenkinsci/gerrit-trigger-plugin/pull/302

          The pull request was merged so I'd say this has been resolved/closed. I'll update the status thanks.

          Show
          tabdy Tyrone Abdy added a comment - https://github.com/jenkinsci/gerrit-trigger-plugin/pull/302 The pull request was merged so I'd say this has been resolved/closed. I'll update the status thanks.

            People

            • Assignee:
              rsandell rsandell
              Reporter:
              tabdy Tyrone Abdy
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: