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

Split the large databound constructors into databound setters

    Details

    • Similar Issues:

      Description

      Inside EC2 plugin there is a lot of constructors with a large set of databound constructors.
      Databound constructor should only be used for mandatory/final parameters

      Where databound setters should be used for optional parameters.

      https://jenkins.io/doc/developer/plugin-development/pipeline-integration/#constructor-vs-setters

        Attachments

          Activity

          Hide
          jvz Matt Sicker added a comment -

          Due to how many people already have pipelines referring to those constructors, it might be worth trying to deprecate that class entirely with a compatibility layer to the new class.

          Show
          jvz Matt Sicker added a comment - Due to how many people already have pipelines referring to those constructors, it might be worth trying to deprecate that class entirely with a compatibility layer to the new class.
          Hide
          raihaan Raihaan Shouhell added a comment -

          We might have to make a breaking change here since everything is public to give jcasc its databinding we need to remove the final keyword but since fields are public they now become directly modifiable. We probably should bite the bullet and change them to be private and provide setters / getters? WDYT: Matt Sicker FABRIZIO MANFREDI

          Show
          raihaan Raihaan Shouhell added a comment - We might have to make a breaking change here since everything is public to give jcasc its databinding we need to remove the final keyword but since fields are public they now become directly modifiable. We probably should bite the bullet and change them to be private and provide setters / getters? WDYT: Matt Sicker FABRIZIO MANFREDI
          Hide
          jvz Matt Sicker added a comment -

          The point of the builder pattern was to allow for a single (private) mega-constructor that continues to change as the class changes, but centralizing the configuration in its own builder class. Alternatively, if all these parameters are moved into their own anemic domain model class, then having everything be set up via getters and setters wouldn't be as painful. The general idea here is about setting up the state of the object by the time it's constructed so that you can avoid representations of invalid state.

          Show
          jvz Matt Sicker added a comment - The point of the builder pattern was to allow for a single (private) mega-constructor that continues to change as the class changes, but centralizing the configuration in its own builder class. Alternatively, if all these parameters are moved into their own anemic domain model class, then having everything be set up via getters and setters wouldn't be as painful. The general idea here is about setting up the state of the object by the time it's constructed so that you can avoid representations of invalid state.
          Hide
          jetersen Joseph Petersen added a comment -

          Why not offer a builder pattern and proper data bound setters, so that those in groovy can use the builder pattern and JCasC and stapler can use data binding.

          Show
          jetersen Joseph Petersen added a comment - Why not offer a builder pattern and proper data bound setters, so that those in groovy can use the builder pattern and JCasC and stapler can use data binding.
          Hide
          jbochenski Jakub Bochenski added a comment -

          Can you please leave the old constructors in place. I'm already using groovys scripts to configure the plugin.
          Splitting out into setters is a welcome change, but it sucks to have to update your scripts for a new release.

          Show
          jbochenski Jakub Bochenski added a comment - Can you please leave the old constructors in place. I'm already using groovys scripts to configure the plugin. Splitting out into setters is a welcome change, but it sucks to have to update your scripts for a new release.

            People

            • Assignee:
              thoulen FABRIZIO MANFREDI
              Reporter:
              casz Joseph Petersen (old)
            • Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated: