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

Mattermost Plugin 2.0+: No field 'showCommitList'

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      Since version 2.0, the Mattermost Plugin no longer has a field showCommitList.
      This leads to Jenkins finding "unreadable data":

      MissingFieldException: No field 'showCommitList' found in class 'jenkins.plugins.mattermost.MattermostNotifier'
      

      Instead, there's now a field commitInfoChoice, which is an enum with (currently) three possible values NONE, AUTHORS, or AUTHORS_AND_TITLES.

      I've also looked at the new generic DSL version, but going from

      mattermost {
      	endpoint(MATTERMOST_HOOK_URL)
      	notifyFailure()
      }
      

      to

      mattermostNotifier {
      	endpoint(MATTERMOST_HOOK_URL)
      	room(null)
      	icon(null)
      	buildServerUrl(null)
      	sendAs(null)
      	startNotification(false)
      	notifyAborted(false)
      	notifyFailure(true)
      	notifyNotBuilt(false)
      	notifySuccess(false)
      	notifyUnstable(false)
      	notifyBackToNormal(false)
      	notifyRepeatedFailure(false)
      	includeTestSummary(false)
      	commitInfoChoice('NONE')
      	includeCustomMessage(false)
      	customMessage(null)
      }
      

      is not exactly an attractive proposition (undocumented, no IDE support, can't leave out default valued fields, very little advantage over a configure block).

        Attachments

          Activity

          hoeferh Henning Hoefer created issue -
          Hide
          daspilker Daniel Spilker added a comment - - edited

          I redirect this to the Mattermost plugin.

          A plugin must not remove a field to retain backwards compatibility, see https://wiki.jenkins-ci.org/display/JENKINS/Hint+on+retaining+backward+compatibility.

          And the automatically generated DSL does require all options because the Mattermost plugin marks the options as mandatory by using a @DataBoundConstructor instead of @DataBoundSetter. This also affects the Pipeline DSL, see https://github.com/jenkinsci/pipeline-plugin/blob/master/DEVGUIDE.md#constructor-vs-setters.

          Show
          daspilker Daniel Spilker added a comment - - edited I redirect this to the Mattermost plugin. A plugin must not remove a field to retain backwards compatibility, see https://wiki.jenkins-ci.org/display/JENKINS/Hint+on+retaining+backward+compatibility . And the automatically generated DSL does require all options because the Mattermost plugin marks the options as mandatory by using a @DataBoundConstructor instead of @DataBoundSetter . This also affects the Pipeline DSL, see https://github.com/jenkinsci/pipeline-plugin/blob/master/DEVGUIDE.md#constructor-vs-setters .
          daspilker Daniel Spilker made changes -
          Field Original Value New Value
          Component/s mattermost-plugin [ 21021 ]
          daspilker Daniel Spilker made changes -
          Assignee Daniel Spilker [ daspilker ] Jo Vandeginste [ jovandeginste ]
          Hide
          jovandeginste Jo Vandeginste added a comment -

          I did some digging, and it seems the slack-plugin I'm basing the mattermost-plugin has this commit:

          https://github.com/jenkinsci/slack-plugin/commit/a0d92735557263c49482103045748a890e05d55e

          Did we forget to include a part from the slack-plugin? Or do (should) they suffer the same problem?

          I'll have a look at the DSL thing.

          Show
          jovandeginste Jo Vandeginste added a comment - I did some digging, and it seems the slack-plugin I'm basing the mattermost-plugin has this commit: https://github.com/jenkinsci/slack-plugin/commit/a0d92735557263c49482103045748a890e05d55e Did we forget to include a part from the slack-plugin? Or do (should) they suffer the same problem? I'll have a look at the DSL thing.
          Hide
          daspilker Daniel Spilker added a comment -

          From what I see in that commit, the Slack plugin should have the same problem. You can simply add a private transient boolean showCommitList field to MattermostNotifier to fix this. This affects all users, not only Job DSL users.

          It would be great if you could change the @DataBoundConstructor to only take the mandatory options and move all optional settings to @DataBoundSetters. I can review your changes to see if it works if you like.

          Show
          daspilker Daniel Spilker added a comment - From what I see in that commit, the Slack plugin should have the same problem. You can simply add a private transient boolean showCommitList field to MattermostNotifier to fix this. This affects all users, not only Job DSL users. It would be great if you could change the @DataBoundConstructor to only take the mandatory options and move all optional settings to @DataBoundSetters . I can review your changes to see if it works if you like.
          Hide
          jovandeginste Jo Vandeginste added a comment -

          That would be really great.

          I added the transient option already (not yet pushed to repository), and am trying to write all the setters. I'm trying to figuring out when the fixNull is relevant – I seem to understand it's not relevant for booleans...

          I'll update this ticket when I push my code to github, and will kindly ask you to review

          Show
          jovandeginste Jo Vandeginste added a comment - That would be really great. I added the transient option already (not yet pushed to repository), and am trying to write all the setters. I'm trying to figuring out when the fixNull is relevant – I seem to understand it's not relevant for booleans... I'll update this ticket when I push my code to github, and will kindly ask you to review
          Hide
          jovandeginste Jo Vandeginste added a comment -

          mvn verify is running, but the changes are in this branch:
          https://github.com/jovandeginste/jenkins-mattermost-plugin/commits/jenkins-39239

          Show
          jovandeginste Jo Vandeginste added a comment - mvn verify is running, but the changes are in this branch: https://github.com/jovandeginste/jenkins-mattermost-plugin/commits/jenkins-39239
          Hide
          jovandeginste Jo Vandeginste added a comment -

          I fixed the failures - the commit url is obviously no longer the same. I appreciate any feedback at this point.

          Show
          jovandeginste Jo Vandeginste added a comment - I fixed the failures - the commit url is obviously no longer the same. I appreciate any feedback at this point.
          Hide
          daspilker Daniel Spilker added a comment -

          Works as expected, nice!

          Show
          daspilker Daniel Spilker added a comment - Works as expected, nice!
          Hide
          jovandeginste Jo Vandeginste added a comment -

          Seriously? This easy...?

          Show
          jovandeginste Jo Vandeginste added a comment - Seriously? This easy...?
          Hide
          daspilker Daniel Spilker added a comment -

          Yup. Merge it, cut a release and done

          Show
          daspilker Daniel Spilker added a comment - Yup. Merge it, cut a release and done
          Hide
          jovandeginste Jo Vandeginste added a comment -

          New release published: 2.3.0 (I ran it in my own environment first to make sure nothing broke)

          Show
          jovandeginste Jo Vandeginste added a comment - New release published: 2.3.0 (I ran it in my own environment first to make sure nothing broke)
          Hide
          jovandeginste Jo Vandeginste added a comment -

          Marking as fixed; if the new version is still missing something, feel free to reopen!

          Show
          jovandeginste Jo Vandeginste added a comment - Marking as fixed; if the new version is still missing something, feel free to reopen!
          jovandeginste Jo Vandeginste made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Resolved [ 5 ]
          daspilker Daniel Spilker made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            Assignee:
            jovandeginste Jo Vandeginste
            Reporter:
            hoeferh Henning Hoefer
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved: