-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[JENKINS-64810] Add creation time boundaries to tags for discovery #1764
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
base: master
Are you sure you want to change the base?
Changes from all commits
5b9acb0
8a7de39
77c1168
d20c7ed
fcfb5f8
df42f96
2dea2d7
ff5e2cd
fa788b3
1c59f53
1abea7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| */ | ||
| package jenkins.plugins.git.traits; | ||
|
|
||
| import edu.umd.cs.findbugs.annotations.CheckForNull; | ||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import hudson.Extension; | ||
| import jenkins.plugins.git.GitSCMBuilder; | ||
|
|
@@ -51,11 +52,20 @@ | |
| * @since 3.6.0 | ||
| */ | ||
| public class TagDiscoveryTrait extends SCMSourceTrait { | ||
| private final String atLeastDays; | ||
| private final String atMostDays; | ||
|
|
||
| /** | ||
| * Constructor for stapler. | ||
| */ | ||
| @DataBoundConstructor | ||
| public TagDiscoveryTrait(@CheckForNull String atLeastDays, @CheckForNull String atMostDays) { | ||
| this.atLeastDays = atLeastDays; | ||
| this.atMostDays = atMostDays; | ||
| } | ||
|
|
||
| public TagDiscoveryTrait() { | ||
| this(null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -66,6 +76,8 @@ | |
| GitSCMSourceContext<?,?> ctx = (GitSCMSourceContext<?, ?>) context; | ||
| ctx.wantTags(true); | ||
| ctx.withAuthority(new TagSCMHeadAuthority()); | ||
| ctx.withAtLeastTagCommitTimeDays(atLeastDays) | ||
| .withAtMostTagCommitTimeDays(atMostDays); | ||
| } | ||
|
Comment on lines
+79
to
81
|
||
|
|
||
| /** | ||
|
|
@@ -76,6 +88,14 @@ | |
| return category instanceof TagSCMHeadCategory; | ||
| } | ||
|
|
||
| public String getAtLeastDays() { | ||
| return atLeastDays; | ||
| } | ||
|
|
||
| public String getAtMostDays() { | ||
| return atMostDays; | ||
| } | ||
|
|
||
| /** | ||
| * Our descriptor. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,10 @@ | ||||||||||||||||||||||||||
| <?jelly escape-by-default='true'?> | ||||||||||||||||||||||||||
| <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:c="/lib/credentials" | ||||||||||||||||||||||||||
| xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"> | ||||||||||||||||||||||||||
| <f:entry field="atLeastDays" title="${%Ignore tags newer than}"> | ||||||||||||||||||||||||||
| <f:number min="0"/> | ||||||||||||||||||||||||||
| </f:entry> | ||||||||||||||||||||||||||
| <f:entry field="atMostDays" title="${%Ignore tags older than}"> | ||||||||||||||||||||||||||
|
Comment on lines
4
to
7
|
||||||||||||||||||||||||||
| <f:entry field="atLeastDays" title="${%Ignore tags newer than}"> | |
| <f:number default=""/> | |
| </f:entry> | |
| <f:entry field="atMostDays" title="${%Ignore tags older than}"> | |
| <f:entry field="atLeastDays" title="${%Discover tags older than (days)}"> | |
| <f:number default=""/> | |
| </f:entry> | |
| <f:entry field="atMostDays" title="${%Discover tags newer than (days)}"> |
Copilot
AI
Jan 23, 2026
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.
The field order in the UI might be counterintuitive. The "atLeastDays" field (tags older than X days) appears before "atMostDays" field (tags newer than Y days). When users think about filtering by time range, they typically think in chronological order: "from X days ago to Y days ago" or in age order: "minimum age to maximum age".
Consider reordering the fields to be more intuitive:
- If using the current labels, swap them so "Ignore tags older than" (atMostDays) comes first, then "Ignore tags newer than" (atLeastDays)
- Or rename to "Minimum tag age" (atLeastDays) first, then "Maximum tag age" (atMostDays)
This would align better with user expectations when setting up a time range filter.
| <f:entry field="atLeastDays" title="${%Ignore tags newer than}"> | |
| <f:number default=""/> | |
| </f:entry> | |
| <f:entry field="atMostDays" title="${%Ignore tags older than}"> | |
| <f:number default="7"/> | |
| </f:entry> | |
| <f:entry field="atMostDays" title="${%Ignore tags older than}"> | |
| <f:number default="7"/> | |
| </f:entry> | |
| <f:entry field="atLeastDays" title="${%Ignore tags newer than}"> | |
| <f:number default=""/> | |
| </f:entry> |
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 doubt this.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| <div> | ||
| Minimum age of tags (in days) to <strong>include</strong> when discovering tags. | ||
| Tags newer than this value are ignored. | ||
| Tag age is calculated from the tag creation / commit timestamp to the time the | ||
| scan runs. | ||
| Leave this field blank to <strong>not</strong> filter out tags based on a minimum age | ||
| (no lower age bound). | ||
| Examples: | ||
| <ul> | ||
| <li><code>7</code> – ignore tags created in the last 7 days (only tags at least 7 days old are used).</li> | ||
| <li><code>0</code> – do not ignore tags for being too new (equivalent to leaving it blank).</li> | ||
| </ul> | ||
| When used together with <em>Ignore tags older than</em> (<code>atMostDays</code>), | ||
| this value should be less than or equal to the <em>older than</em> value to define a valid age range. | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <div> | ||
| Maximum age of tags (in days) to <strong>include</strong> when discovering tags. | ||
| Tags older than this value are ignored. | ||
| Tag age is calculated from the tag creation / commit timestamp to the time the | ||
| scan runs. | ||
| Leave this field blank to <strong>not</strong> filter out tags based on a maximum age | ||
| (no upper age bound). | ||
| Examples: | ||
| <ul> | ||
| <li><code>7</code> – ignore tags older than 7 days (only tags from the last 7 days are used).</li> | ||
| <li><code>30</code> – ignore tags older than 30 days (only tags from the last 30 days are used).</li> | ||
| </ul> | ||
| When used together with <em>Ignore tags newer than</em> (<code>atLeastDays</code>), | ||
| this value should be greater than or equal to the <em>newer than</em> value so that the | ||
| age range is consistent. | ||
| </div> |
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.
The filtering logic checks if atMostMillis >= 0L && atLeastMillis > atMostMillis and continues to skip the tag if this invalid configuration is detected. However, this check is performed inside the loop for every tag, which is inefficient.
Consider moving this validation earlier, such as:
This would provide earlier feedback to users about configuration errors and avoid redundant checks for every tag.
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 don't think this is necessary.