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

Declarative should provide an easy way to disable/enable some steps in an PR context or not

      When writing a Jenkinsfile, if you have email notifications, or notifications in general, this probably does not make sense to send those when building a PR. Same goes for actual deployments or touching real environments by any mean.

      PR are reviewed through GitHub, have a build status there, and so on. So by default, sending emails to maintainers creates unnecessary noise.

      Maybe this be globally enabled/disabled by default though the new options block.

      Like:

      options {
         notifyOnChangeRequest = true // I would argue that we should be opinionated and put it with a default of false
      }
      

      And possibly be overridable per step, like:

      notOnChangeRequest { // PR on GitHub & Bitbucket, patchset on Gerrit, Review Request in ReviewBoard...
         sh "deploy-to-production.sh"
      }
      

      For when and usual stage deactivations:

      when {
          branch CHANGE_REQUEST
      }
      

      WDYT?

      Thanks!

          [JENKINS-40898] Declarative should provide an easy way to disable/enable some steps in an PR context or not

          Also:

          Hmmm. So the when condition on stages is probably what you need, but you'd have to put your notification(s) in a separate stage since when controls whether the entire stage executes. I'm also not sure how you'd determine if you're in a PR context. =)

          As discussed on IRC, won't work because if the pipeline fails in early stages, then the stage with those notifications won't be reached anyway, hence the notifications never sent.

          Baptiste Mathus added a comment - Also: Hmmm. So the when condition on stages is probably what you need, but you'd have to put your notification(s) in a separate stage since when controls whether the entire stage executes. I'm also not sure how you'd determine if you're in a PR context. =) As discussed on IRC, won't work because if the pipeline fails in early stages, then the stage with those notifications won't be reached anyway, hence the notifications never sent.

          Patrick Wolf added a comment - - edited

          A Pipeline step called notOnChangeRequest or skipOnPR that will wrap steps to skip in PR build would work in Declarative without needing script. It would also be available to use in Scripted Pipeline. A step that simple could also be implemented in a Shared Library.

          Having a global option of notifyOnChangeRequest means we have to maintain a static list of notifying steps to blacklist.

          Patrick Wolf added a comment - - edited A Pipeline step called notOnChangeRequest or skipOnPR that will wrap steps to skip in PR build would work in Declarative without needing script . It would also be available to use in Scripted Pipeline. A step that simple could also be implemented in a Shared Library. Having a global option of notifyOnChangeRequest means we have to maintain a static list of notifying steps to blacklist.

          Andrew Bayer added a comment -

          I think when branch does the trick here?

          Andrew Bayer added a comment - I think when branch does the trick here?

          OMG missed that.
          abayer can one use branch in a post/failure for instance?

          Baptiste Mathus added a comment - OMG missed that. abayer can one use branch in a post/failure for instance?

          Andrew Bayer added a comment -

          No - open a ticket for that - I know someone else requested something along those lines.

          Andrew Bayer added a comment - No - open a ticket for that - I know someone else requested something along those lines.

          Patrick Wolf added a comment -

          Could also have a stage called "notifications" at the end of your pipeline.

          Patrick Wolf added a comment - Could also have a stage called "notifications" at the end of your pipeline.

          hrmpw well, nope

          Baptiste Mathus added a comment - hrmpw well, nope

          OMG, crappy cross comment linking. I was referring to my previous comment about this:

          As discussed on IRC, won't work because if the pipeline fails in early stages, then the stage with those notifications won't be reached anyway, hence the notifications never sent.

          Baptiste Mathus added a comment - OMG, crappy cross comment linking. I was referring to my previous comment about this: As discussed on IRC, won't work because if the pipeline fails in early stages, then the stage with those notifications won't be reached anyway, hence the notifications never sent.

          abayer just filed JENKINS-42688

          Baptiste Mathus added a comment - abayer just filed JENKINS-42688

          Liam Newman added a comment -

          Bulk closing resolved issues.

          Liam Newman added a comment - Bulk closing resolved issues.

            abayer Andrew Bayer
            batmat Baptiste Mathus
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: