Skip to content

Use custom detailsURL in checks when configured#3354

Open
esatterwhite wants to merge 1 commit into
jenkinsci:mainfrom
answerbook:esatterwhite/custom-details-url
Open

Use custom detailsURL in checks when configured#3354
esatterwhite wants to merge 1 commit into
jenkinsci:mainfrom
answerbook:esatterwhite/custom-details-url

Conversation

@esatterwhite

Copy link
Copy Markdown

This should make it possible to specify a detailsURL property to the recordIssues and publishIssues step functions.
When issues are published via the checks api - this details url is hard coded to point to the jenkins ui for the build in question.
in cases when jenkins isn't exposed to the public, this link causes a lot of confusion.

In particular if running jenkins against github repos, and jenkins isn't accessible to the public, these links are also not accessible. This aims to provide a way to provide a custom url that to change the link so the link points to to something that is publically available.

I tried to include applicable tests to validate that the changes were in doing what I think / expect them to. making sure the defaults case still produces the expected url, and that providing an override is honored.

Resolves: #3352

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests that demonstrate the feature works or the issue is fixed

this piggybacks on jenkinsci/checks-api-plugin#329 which mad it possible
to include a custom details url on the checks info. This change should
make it possible to either carry the custom url if defined through to
sub steps of withChecks. Additionally, the publishChecks and
recordIssues steps will accept a details url directly when called
independantly of `withChecks` in which case they will update the
checksinfo as well.

Resolves: jenkinsci#3352
@github-actions
github-actions Bot requested a review from uhafner June 11, 2026 19:25
@esatterwhite

Copy link
Copy Markdown
Author

supersedes: #3353

Comment thread plugin/pom.xml
@CheckForNull
private ChecksInfo checksInfo;

@CheckForNull

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@CheckForNull probably not necessary

private String icon = StringUtils.EMPTY; // @since 12.0.0: by default no custom icon is set
private String scm = StringUtils.EMPTY;

@CheckForNull

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@CheckForNull probably not necessary

private boolean stopBuild; // @since 12.10010: by default, pipeline execution will not be stopped
private String scm = StringUtils.EMPTY;

@CheckForNull

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

probably not necessary

@uhafner uhafner changed the title feat: allow custom detailsURL in recordIssues and publishChecks Use custom detailsURL in checks when configured Jun 12, 2026
@uhafner uhafner added the enhancement Enhancement of existing functionality label Jun 12, 2026

@uhafner uhafner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wasn't the idea in jenkinsci/checks-api-plugin#328 that no configuration is required in a plugin anymore? I.e. we only need to change the code in WarningsChecksPublisher to extract the new property from the ChecksInfo class. There should be no new property in the plugins itself?

I.e., with jenkinsci/checks-api-plugin#329 you can run the step:

steps {
     withChecks(name: 'test', detailsURL: 'http://codedependant.net') {
             recordTests ... 
             recordCoverage ...
             recordIssues ...
     }
}

Comment thread plugin/pom.xml
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>checks-api</artifactId>
<version>415.vf022234a_931d</version>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<version>415.vf022234a_931d</version>
<!-- TODO remove explicit version once it's available in BOM -->
<version>415.vf022234a_931d</version>

Comment thread plugin/pom.xml
@github-actions

Copy link
Copy Markdown

☀️   Quality Monitor

Tests

   JUnit   Unit Tests: ✅ successful — 92 passed $\textsf{(±0)}$, 2 skipped $\textsf{(±0)}$
   🚀   Integration Tests: ✅ successful — 515 passed $\color{green}{\textsf{(+7)}}$, 10 skipped $\textsf{(±0)}$
   ⛔   Architecture Tests: ✅ successful — 15 passed $\textsf{(±0)}$

Coverage for New Code

   〰️   Line Coverage: 95.65% — 1 missed lines
   ➰   Branch Coverage: 100.00% — perfect 🎉

Coverage for Whole Project

   〰️   Line Coverage: 80.85% $\color{red}{\textsf{(-0.02)}}$ — 1193 missed lines
   ➰   Branch Coverage: 65.18% $\color{red}{\textsf{(-0.07)}}$ — 475 missed branches

Style

   CheckStyle   CheckStyle: No warnings $\textsf{(±0)}$
   PMD   PMD: No warnings $\textsf{(±0)}$
   ☕   Java Compiler: 16 warnings $\textsf{(±0)}$ — normal: 16

Bugs

   SpotBugs   SpotBugs: No bugs $\textsf{(±0)}$
   🐛   Error Prone: No bugs $\textsf{(±0)}$

Vulnerabilities

   🛡️   OWASP Dependency Check: 276 vulnerabilities $\color{green}{\textsf{(+276)}}$ — error: 28, high: 84, normal: 153, low: 11

Software Metrics

   🌀   Cyclomatic Complexity: 2644 (total)
   💭   Cognitive Complexity: 855 (total)
   ➿   N-Path Complexity: 3023 (total)
   📏   Lines of Code: 32215 (total)
   📝   Non Commenting Source Statements: 10706 (total)
   🔗   Class Cohesion: 100.00% (maximum)
   ⚖️   Weight of Class: 100.00% (maximum)

📌 Reference Results

Delta reports computed against the reference results of be73fb8 in workflow run 26680622135.

🚦 Quality Gates

Overall Status: ❌ FAILURE

✅ Passed Gates

  • ✅ Overall Tests Success Rate: 100.00 >= 100.00
  • ✅ Line Coverage in New Code: 95.65 >= 90.00
  • ✅ Branch Coverage in New Code: 100.00 >= 90.00
  • ✅ Potential Bugs in Whole Project: 0.00 <= 0.00

❌ Failed Gates

  • ❌ Style Violation in Whole Project: 16.00 <= 0.00

Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result.

@esatterwhite

Copy link
Copy Markdown
Author

Wasn't the idea in jenkinsci/checks-api-plugin#328 that no configuration is required in a plugin anymore? I.e. we only need to change the code in WarningsChecksPublisher to extract the new property from the ChecksInfo class. There should be no new property in the plugins itself?

I.e., with jenkinsci/checks-api-plugin#329 you can run the step:

steps {
     withChecks(name: 'test', detailsURL: 'http://codedependant.net') {
             recordTests ... 
             recordCoverage ...
             recordIssues ...
     }
}

Thats is an option. It seemed reasonable to me to be able to do the same thing if I wasn't using withChecks specifically. and just calling the issues functions directly

recordIssues(
 ...
, detailsURL: "http://example.com/issues"
)

turtles all the way down I guess.

@uhafner

uhafner commented Jun 18, 2026

Copy link
Copy Markdown
Member

Wasn't the idea in jenkinsci/checks-api-plugin#328 that no configuration is required in a plugin anymore? I.e. we only need to change the code in WarningsChecksPublisher to extract the new property from the ChecksInfo class. There should be no new property in the plugins itself?
I.e., with jenkinsci/checks-api-plugin#329 you can run the step:

steps {
     withChecks(name: 'test', detailsURL: 'http://codedependant.net') {
             recordTests ... 
             recordCoverage ...
             recordIssues ...
     }
}

Thats is an option. It seemed reasonable to me to be able to do the same thing if I wasn't using withChecks specifically. and just calling the issues functions directly

recordIssues(
 ...
, detailsURL: "http://example.com/issues"
)

turtles all the way down I guess.

Hmm, this causes a lot of code duplication in a lot of plugins just to provide a convenience call. I'm not sure if it is worth the effort. What do you think @timja? This would imply changes in all those reporting plugins like JUnit, etc.

@esatterwhite

Copy link
Copy Markdown
Author

Wasn't the idea in jenkinsci/checks-api-plugin#328 that no configuration is required in a plugin anymore? I.e. we only need to change the code in WarningsChecksPublisher to extract the new property from the ChecksInfo class. There should be no new property in the plugins itself?
I.e., with jenkinsci/checks-api-plugin#329 you can run the step:

steps {
     withChecks(name: 'test', detailsURL: 'http://codedependant.net') {
             recordTests ... 
             recordCoverage ...
             recordIssues ...
     }
}

Thats is an option. It seemed reasonable to me to be able to do the same thing if I wasn't using withChecks specifically. and just calling the issues functions directly

recordIssues(
 ...
, detailsURL: "http://example.com/issues"
)

turtles all the way down I guess.

Hmm, this causes a lot of code duplication in a lot of plugins just to provide a convenience call. I'm not sure if it is worth the effort. What do you think @timja? This would imply changes in all those reporting plugins like JUnit, etc.

@uhafner Thoughts? I'm willing to work my way through the reporting plugins if others are not.

@timja

timja commented Jul 6, 2026

Copy link
Copy Markdown
Member

Hmm, this causes a lot of code duplication in a lot of plugins just to provide a convenience call. I'm not sure if it is worth the effort. What do you think @timja? This would imply changes in all those reporting plugins like JUnit, etc.

Sorry missed this, I would expect the ChecksContext to be read and used in each plugin if its set but changes should be minimal and shouldn't need config on each step.

@uhafner

uhafner commented Jul 7, 2026

Copy link
Copy Markdown
Member

I would expect the ChecksContext to be read and used in each plugin if its set but changes should be minimal and shouldn't need config on each step.

This is also my impression. So @esatterwhite, can you change the implementation so that only the new checks API object is read? Then we need no configuration in the warnings plugin.

(If this does not work out we can still create a followup that adds a new property).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

custom detailsURL for recordIssues and recordCoverage

3 participants