-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add withChecks step #49
Merged
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
dad6572
Add withChecks step
XiongKezhi 7b78a3b
Add test for WithChecksStep.
XiongKezhi 80e6a26
Add document and snippet generator support.
XiongKezhi f4b246a
Bump version to 1.2.0 for adding ChecksInfo and WithChecksStep.
XiongKezhi 637b847
Merge branch 'master' into implement_with_checks_step
XiongKezhi c76461f
remove unused setter
XiongKezhi d37be34
Remove unused import
XiongKezhi 69a09ff
Add callback for withChecks.
XiongKezhi d38dbf6
Add consumer doc.
XiongKezhi a45182e
Remove unnessary denpendency version in pom
XiongKezhi e724c04
Improve consumer doc
XiongKezhi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
17 changes: 17 additions & 0 deletions
17
src/main/java/io/jenkins/plugins/checks/steps/ChecksInfo.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package io.jenkins.plugins.checks.steps; | ||
|
||
import java.io.Serializable; | ||
|
||
public class ChecksInfo implements Serializable { | ||
private static final long serialVersionUID = 1L; | ||
|
||
private String name; | ||
|
||
public ChecksInfo(String name) { | ||
this.name = name; | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
} |
73 changes: 73 additions & 0 deletions
73
src/main/java/io/jenkins/plugins/checks/steps/WithChecksStep.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
package io.jenkins.plugins.checks.steps; | ||
|
||
import hudson.Extension; | ||
import hudson.model.Run; | ||
import hudson.model.TaskListener; | ||
import org.jenkinsci.plugins.workflow.steps.*; | ||
import org.kohsuke.stapler.DataBoundConstructor; | ||
|
||
import java.io.Serializable; | ||
import java.util.*; | ||
|
||
public class WithChecksStep extends Step implements Serializable { | ||
private static final long serialVersionUID = 1L; | ||
|
||
private String name; | ||
|
||
/** | ||
* Constructor used for pipeline by Stapler. | ||
*/ | ||
@DataBoundConstructor | ||
public WithChecksStep(final String name) { | ||
super(); | ||
|
||
this.name = name; | ||
} | ||
|
||
@Override | ||
public StepExecution start(final StepContext stepContext) { | ||
return new WithChecksStep.WithChecksStepExecution(stepContext, this); | ||
} | ||
|
||
@Extension | ||
public static class WithChecksStepDescriptor extends StepDescriptor { | ||
@Override | ||
public String getFunctionName() { | ||
return "withChecks"; | ||
} | ||
|
||
@Override | ||
public Set<? extends Class<?>> getRequiredContext() { | ||
return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(Run.class, TaskListener.class))); | ||
} | ||
|
||
@Override | ||
public boolean takesImplicitBlockArgument() { | ||
return true; | ||
} | ||
} | ||
|
||
static class WithChecksStepExecution extends AbstractStepExecutionImpl { | ||
private WithChecksStep step; | ||
|
||
WithChecksStepExecution(final StepContext context, final WithChecksStep step) { | ||
super(context); | ||
this.step = step; | ||
} | ||
|
||
@Override | ||
public boolean start() { | ||
StepContext context = getContext(); | ||
context.newBodyInvoker() | ||
.withContext(new ChecksInfo(step.name)) | ||
.withCallback(BodyExecutionCallback.wrap(context)) | ||
.start(); | ||
return false; | ||
} | ||
|
||
@Override | ||
public void stop(Throwable cause) { | ||
getContext().onFailure(cause); | ||
} | ||
} | ||
} |
8 changes: 8 additions & 0 deletions
8
src/main/java/io/jenkins/plugins/checks/steps/package-info.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/** | ||
* Provides default Findbugs annotations. | ||
*/ | ||
@DefaultAnnotation(NonNull.class) | ||
package io.jenkins.plugins.checks.steps; | ||
|
||
import edu.umd.cs.findbugs.annotations.DefaultAnnotation; | ||
import edu.umd.cs.findbugs.annotations.NonNull; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to take more than one args? cause I saw code like https://github.com/jenkinsci/workflow-cps-plugin/blob/master/src/main/java/org/jenkinsci/plugins/workflow/cps/DSL.java#L702
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it can take more than one, normally you would use a databoundsetter for any optional parameters
Then the syntax would be:
if there's only one mandatory field in constructor then you can omit the parameter name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So should we make all params optional in case that in the future, some users may only want to inject a certain field like conclusion instead of name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we do that then the user will have to specify the name in a more verbose way,
withChecks(name: 'hello')
rather thanwithChecks('hello')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify the name wouldn't be annoying for users IMO but make it mandatory will be an issue for future changes?
Also, it seems the snippet generator will always specify name
@mrginglymus What do you think? Do you think there will be situations where you do not want to specify the name but the status (e.g. publish several checks in the same status (say in progress) but with different names in the closure)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be fine to be mandatory