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

job-dsl-plugin Still Uses Mattermost showCommitList

      The job-dsl-plugin still refers to the Mattermost plugin's old showCommitList() functionality.  This was replaced in Mattermost Plugin 2.0 with commitInfoChoice(), and referred to under JENKINS-39239.  Jobs built with the DSL plugin's helpers for the Mattermost plugin are unable to set commitInfoChoice().  I'm opening a pull request back to the DSL repo with a fix for this.

          [JENKINS-42887] job-dsl-plugin Still Uses Mattermost showCommitList

          This is supported by the Automatically Generated DSL:

          job(String name) {
            publishers {
              mattermostNotifier {
                endpoint(String value)
                buildServerUrl(String value)
                commitInfoChoice(String value)
                customMessage(String value)
                icon(String value)
                includeCustomMessage(boolean value)
                includeTestSummary(boolean value)
                notifyAborted(boolean value)
                notifyBackToNormal(boolean value)
                notifyFailure(boolean value)
                notifyNotBuilt(boolean value)
                notifyRepeatedFailure(boolean value)
                notifySuccess(boolean value)
                notifyUnstable(boolean value)
                room(String value)
                sendAs(String value)
                startNotification(boolean value)
              }
            }
          }

           

          Daniel Spilker added a comment - This is supported by the Automatically Generated DSL : job( String name) { publishers { mattermostNotifier { endpoint( String value) buildServerUrl( String value) commitInfoChoice( String value) customMessage( String value) icon( String value) includeCustomMessage( boolean value) includeTestSummary( boolean value) notifyAborted( boolean value) notifyBackToNormal( boolean value) notifyFailure( boolean value) notifyNotBuilt( boolean value) notifyRepeatedFailure( boolean value) notifySuccess( boolean value) notifyUnstable( boolean value) room( String value) sendAs( String value) startNotification( boolean value) } } }  

          Jayme Howard added a comment -

          You're correct, I agree.  However, if direct support for Mattermost exists in the job-dsl-plugin, shouldn't it be correct and accurate?  Further, using automatically generated DSL isn't an option for all users.  In my case, we've extended HMRC's library to sit on top of the plugin, to do factory based job generation and be able to test our jobs offline.  This requires calling the backing methods directly, or implementing everything within configure blocks.  I don't expect the plugin to support every user's weird use case, but the hooks for support already exist and were accepted into the codebase, so keeping them accurate feels worthwhile to me, and I've written the patch with that in mind.

           

          I do see now that you've opened a merge request with intent to deprecate the Mattermost support.  If that's the intended direction, I can go ahead and close my merge request and this issue, and rewrite my implementation via configure.

          Jayme Howard added a comment - You're correct, I agree.  However, if direct support for Mattermost exists in the job-dsl-plugin, shouldn't it be correct and accurate?  Further, using automatically generated DSL isn't an option for all users.  In my case, we've extended HMRC's library to sit on top of the plugin, to do factory based job generation and be able to test our jobs offline.  This requires calling the backing methods directly, or implementing everything within configure blocks.  I don't expect the plugin to support every user's weird use case, but the hooks for support already exist and were accepted into the codebase, so keeping them accurate feels worthwhile to me, and I've written the patch with that in mind.   I do see now that you've opened a merge request with intent to deprecate the Mattermost support.  If that's the intended direction, I can go ahead and close my merge request and this issue, and rewrite my implementation via configure.

          I'm deprecating support for plugins that became incompatible and that can be replaced by the Automatically Generated DSL. It's too cumbersome to keep up with all plugin changes. I will only keep plugins that stay compatible with older version or that can not easily be migrated to the Automatically Generated DSL.

          Daniel Spilker added a comment - I'm deprecating support for plugins that became incompatible and that can be replaced by the Automatically Generated DSL. It's too cumbersome to keep up with all plugin changes. I will only keep plugins that stay compatible with older version or that can not easily be migrated to the Automatically Generated DSL.

            daspilker Daniel Spilker
            isugimpy Jayme Howard
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: