Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JobPropertyStep #209

Merged
merged 14 commits into from Oct 28, 2015

Conversation

Projects
None yet
4 participants
@jglick
Copy link
Member

jglick commented Sep 17, 2015

JENKINS-30519

  • create JobPropertyStep
  • fix or work around JENKINS-26535 problem in DescribableHelper
  • work around lack of @DataBoundConstructor on ParametersDefinitionProperty
  • figure out how to deal with the nightmare that is the design of JobProperty/config.jelly
  • fix JENKINS-29711 problem in Snippetizer (leave it for another day)
  • delete WorkflowParameterDefinitionBranchProperty
  • buildDiscarder
  • triggers (not now)
  • quietPeriod (not now)
  • concurrentBuild (not now)
  • authToken (not now)
  • suppress RateLimitBranchProperty
  • test against jenkinsci/github-plugin#84 when released (have tested against snapshots)
  • move OptionalJobProperty upstream: #228
  • move BuildDiscarderProperty upstream: #229

@reviewbybees esp. @kohsuke & @stephenc

@reviewbybees

This comment has been minimized.

Copy link

reviewbybees commented Sep 17, 2015

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@Extension public static class HideSuperfluousBranchProperties extends DescriptorVisibilityFilter {

@Override public boolean filter(Object context, Descriptor descriptor) {
if (context instanceof WorkflowMultiBranchProject && (descriptor.clazz == RateLimitBranchProperty.class || descriptor.clazz == BuildRetentionBranchProperty.class)) {

This comment has been minimized.

Copy link
@amuniz

amuniz Oct 27, 2015

Member

It seems hacky? If another property like RateLimitBranchProperty is added for some reason, it should be excluded here too. Is not there another way to exclude them?

This comment has been minimized.

Copy link
@jglick

jglick Oct 27, 2015

Author Member

We want to include unknown branch properties by default. There is no way that I can think of to mechanically determine whether they are simply wrappers for job properties.

sampleRepo.init();
ScriptApproval.get().approveSignature("method groovy.lang.Binding hasVariable java.lang.String"); // TODO add to generic whitelist
sampleRepo.write("Jenkinsfile",
"properties([[$class: 'ParametersDefinitionProperty', parameterDefinitions: [[$class: 'StringParameterDefinition', name: 'myparam', defaultValue: 'default value']]]])\n" +

This comment has been minimized.

Copy link
@amuniz

amuniz Oct 27, 2015

Member

I know it's what we have for now, but I have to say this is the longest way to define a key=value pair I have ever seen in my life.

This comment has been minimized.

Copy link
@jglick

jglick Oct 27, 2015

Author Member

JENKINS-29922 should help somewhat. Beyond that, we are stuck with what Jenkins has defined historically (long before code-as-config was a consideration). Creating a special “snowflake” syntax for each known job property will not scale.

jglick added a commit that referenced this pull request Oct 28, 2015

@jglick jglick merged commit 9a28e9c into jenkinsci:master Oct 28, 2015

1 check passed

Jenkins This pull request looks good
Details

@jglick jglick deleted the jglick:JobPropertyStep branch Oct 28, 2015

@KostyaSha

This comment has been minimized.

jglick added a commit to jglick/workflow-multibranch-plugin that referenced this pull request May 25, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.