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.

Sign up for GitHub

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

[JENKINS-49488] implement any_met_condition flag for PromotionProcess.isMet() #115

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[JENKINS-49488] implement any_met_condition flag for PromotionProcess.isMet() #115

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eadb5b2
JENKINS-49488: implement any_met_condition flag for PromotionProcess.…
Mar 23, 2018
1811606
Remove invalid logging
Aug 17, 2018
b2d61fb
Change public any_met_condition to private and create getter function…
Aug 17, 2018
6f31e50
Rename any_met_condition to anyMetCondition to mett Java convention
Aug 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/main/java/hudson/plugins/promoted_builds/PromotionProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import java.util.List;
import java.util.Set;
import java.util.concurrent.Future;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Pattern;
import javax.annotation.CheckForNull;
Expand Down Expand Up @@ -90,6 +91,11 @@ public final class PromotionProcess extends AbstractProject<PromotionProcess,Pro
*/
public String isVisible;

/**
* Trigger promotion when any criteria condition is met.
*/
public boolean any_met_condition;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would advice against introducing any more public fields in the plugin

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I use protected or private here?
I recall I hit an issue before when not using public.


private List<BuildStep> buildSteps = new ArrayList<BuildStep>();

/*package*/ PromotionProcess(JobPropertyImpl property, String name) {
Expand Down Expand Up @@ -144,6 +150,7 @@ public void doSetName(String name) {
assignedLabel = null;
}
isVisible = c.getString("isVisible");
any_met_condition = c.optBoolean("any_met_condition");
save();
}

Expand Down Expand Up @@ -372,11 +379,21 @@ public Status isMet(AbstractBuild<?,?> build) {
List<PromotionBadge> badges = new ArrayList<PromotionBadge>();
for (PromotionCondition cond : conditions) {
PromotionBadge b = cond.isMet(this, build);
if(b==null)
return null;
badges.add(b);
LOGGER.log(Level.INFO, "PromotionProcess.isMet(): any_met_condition={0}", this.any_met_condition);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logging does not help - PromotionProcess is not references in the entry

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll remove this line

if (this.any_met_condition) {
if (b!=null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Since you're modifying these two if statements anyway, I think it would be good to go ahead and use braces around the conditional statements.

badges.add(b);
} else {
if(b==null)
return null;
badges.add(b);
}
}
if (badges.isEmpty()) {
return null;
} else {
return new Status(this,badges);
}
return new Status(this,badges);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/hudson/plugins/promoted_builds/PromotionProcess/process-config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@
</select>
</f:entry>

<f:entry title="Any_Met">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what shows up in the UI? Maybe something like Promote if ANY of the above conditions are met would be more clear to users, but I'm not sure what would be best.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GUI looks like this:
Due to the alignment, I can't have long string at the title.
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the screenshot! Maybe use Any Met with no underscore?

<f:checkbox name="any_met_condition" title="${%Trigger promotion when any criteria condition is met}"
checked="${instance.any_met_condition}"/>
</f:entry>

<f:optionalBlock name="hasAssignedLabel" title="${%Restrict where this promotion process can be run}"
checked="${instance.assignedLabelString!=null}" inline="true">
<f:entry title="${%Label Expression}"
Expand Down