Skip to content

Conversation

@janfaracik
Copy link
Member

@janfaracik janfaracik commented Dec 16, 2025

Still rough, tests will definitely be broken:

image image

Would be good to get thoughts.

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@timja
Copy link
Member

timja commented Dec 16, 2025

Before screenshots would be great to add to make easier for comparison

</l:hasAdministerOrManage>
</f:form>
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:f="/lib/form" xmlns:i="jelly:fmt" xmlns:t="/lib/hudson">
<l:settings-subpage permissions="${app.MANAGE_AND_SYSTEM_READ}" header="${null}">
Copy link
Member

Choose a reason for hiding this comment

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

why pass header=null? you can just pass your own header through with a j:set ?

<button class="jenkins-button jenkins-button--primary"
data-type="dialog-opener"
data-dialog-id="${dialogId}">
Apply Configuration as Code file
Copy link
Member

@timja timja Dec 16, 2025

Choose a reason for hiding this comment

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

i18n and maybe

Suggested change
Apply Configuration as Code file
${%Setup configuration}


<l:app-bar title="${it.displayName}">
<j:if test="${!empty it.sources}">
<button class="jenkins-button jenkins-button--primary"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this needs / wants to be primary. It should be very rare that you're adding a config through the UI...

<gitHubRepo>jenkinsci/configuration-as-code-plugin</gitHubRepo>
<jenkins.baseline>2.504</jenkins.baseline>
<jenkins.version>${jenkins.baseline}.3</jenkins.version>
<jenkins.version>2.542-rc37822.b_14a_5941318e</jenkins.version>
Copy link
Member

Choose a reason for hiding this comment

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

what's this need an incremental for?

@timja
Copy link
Member

timja commented Dec 16, 2025

tests will definitely be broken

I'd be surprised if there's much test coverage around this page...

export(out);

req.setAttribute("export", out.toString(StandardCharsets.UTF_8.name()));
req.setAttribute("viewExport", new ManagementLink() {
Copy link
Member

Choose a reason for hiding this comment

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

I've been doing a dirty in Credentials plugin and just using a class that has a displayName...

@timja
Copy link
Member

timja commented Dec 19, 2025

I've tested locally and it looks great.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants