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.

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-56951] Fix folder property layout #187

Merged
merged 2 commits into from Apr 30, 2019
Merged

[JENKINS-56951] Fix folder property layout #187

merged 2 commits into from Apr 30, 2019

Conversation

alecharp
Copy link
Member

@alecharp alecharp commented Apr 29, 2019

issue: JENKINS-56951

When the folder properties list is built, it's already under a titled section see
https://github.com/jenkinsci/cloudbees-folder-plugin/blob/9d87f2f197554d93a97422a010bd92087e4e576b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly#L136

Adding the section title in the JiraFolderProperty configuration file broken the layout
and let to think that the other properties (from other plugins) could be part of the Jira
section as well.

Removing the title here fix that.

When the folder properties list is built, it's already under a titled section see
https://github.com/jenkinsci/cloudbees-folder-plugin/blob/9d87f2f197554d93a97422a010bd92087e4e576b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/configure.jelly#L136

Adding the section title in the JiraFolderProperty configuration file broken the layout
and let to think that the other properties (from other plugins) could be part of the Jira 
section as well.

Removing the title here fix that.
@coveralls
Copy link

coveralls commented Apr 29, 2019

Coverage Status

Coverage remained the same at 55.481% when pulling 74e8052 on alecharp:JENKINS-56951 into 0d75ad8 on jenkinsci:master.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Why not just delete the f:section altogether?

@alecharp
Copy link
Member Author

@jglick you are right, I could, the behavior is the same. Just that having a section can be seen as a "scope" for the entry. Would it be better without?

@jglick
Copy link
Member

jglick commented Apr 29, 2019

Would it be better without?

I think so.

As suggested by @jglick, removing the `f:section` entirely to fix the folder property layout
@alecharp
Copy link
Member Author

Thanks @jglick for your time, I included your suggestion.

@artkoshelev artkoshelev merged commit 83fc8c3 into jenkinsci:master Apr 30, 2019
@batmat
Copy link
Member

batmat commented Apr 30, 2019

@artkoshelev @olamy could a release be possibly considered?

I see there are a handful of other unreleased improvements in master, like the JCasC support in #168 among others.
Release early, often, and keep changes small ;-).

Thanks!

@olamy
Copy link
Member

olamy commented Apr 30, 2019 via email

@batmat
Copy link
Member

batmat commented Apr 30, 2019

Would be perfect, thanks Olivier!

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.

None yet

7 participants