-
Notifications
You must be signed in to change notification settings - Fork 745
Adjust for experimental Manage Jenkins UI #2756
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?
Conversation
|
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}"> |
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.
why pass header=null? you can just pass your own header through with a j:set ?
plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCode/index.jelly
Outdated
Show resolved
Hide resolved
| <button class="jenkins-button jenkins-button--primary" | ||
| data-type="dialog-opener" | ||
| data-dialog-id="${dialogId}"> | ||
| Apply Configuration as Code file |
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.
i18n and maybe
| 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" |
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 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> |
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.
what's this need an incremental for?
I'd be surprised if there's much test coverage around this page... |
plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCode/index.jelly
Show resolved
Hide resolved
plugin/src/main/resources/io/jenkins/plugins/casc/ConfigurationAsCode/index.jelly
Outdated
Show resolved
Hide resolved
| export(out); | ||
|
|
||
| req.setAttribute("export", out.toString(StandardCharsets.UTF_8.name())); | ||
| req.setAttribute("viewExport", new ManagementLink() { |
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've been doing a dirty in Credentials plugin and just using a class that has a displayName...
|
I've tested locally and it looks great. |
Still rough, tests will definitely be broken:
Would be good to get thoughts.
Your checklist for this pull request
🚨 Please review the guidelines for contributing to this repository.