Introduce Named, FullyNamed, FullyNamedModelObject interfaces#10827
Introduce Named, FullyNamed, FullyNamedModelObject interfaces#10827MarkEWaite merged 8 commits intojenkinsci:masterfrom
Named, FullyNamed, FullyNamedModelObject interfaces#10827Conversation
…erfaces These correspond to methods `getName`, `getFullName`, `getFullDisplayName` that did not have any common interface to call them, causing some inconvenient usages of switch statements or even reflection in CloudBees CI HA.
Nameable, FullyNameable, FullyNameableModelObject interfacesNamed, FullyNamed, FullyNamedModelObject interfaces
| // TODO review usage of this method and replace with getFullDisplayName() where appropriate | ||
| return getTask().getFullDisplayName(); |
There was a problem hiding this comment.
Not sure I understand the comment. Did you mean
| // TODO review usage of this method and replace with getFullDisplayName() where appropriate | |
| return getTask().getFullDisplayName(); | |
| return getTask().getDisplayName(); |
? And should ResourceActivity implement ModelObject?
There was a problem hiding this comment.
The name of the method implies this would be the natural implementation, but since it actually uses the full display name, these could cause some UI inconsistencies if changed without reviewing all current usages.
There was a problem hiding this comment.
And should ResourceActivity implement ModelObject?
I had missed this interface, I guess it should, for consistency with ITask
There was a problem hiding this comment.
So do I understand the comment to mean that you propose switching QueueItem.displayName to use Task.displayName but looking up existing callers (probably in Jelly) and switching them from ${i.displayName} to ${i.fullDisplayName} where appropriate? If so, that was not clear to me from the comment text.
Co-authored-by: Jesse Glick <jglick@cloudbees.com>
| // TODO review usage of this method and replace with getFullDisplayName() where appropriate | ||
| return getTask().getFullDisplayName(); |
There was a problem hiding this comment.
So do I understand the comment to mean that you propose switching QueueItem.displayName to use Task.displayName but looking up existing callers (probably in Jelly) and switching them from ${i.displayName} to ${i.fullDisplayName} where appropriate? If so, that was not clear to me from the comment text.
NotMyFault
left a comment
There was a problem hiding this comment.
/label ready-for-merge
This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback.
Please see the merge process documentation for more information about the merge process.
Thanks!
Fix two tests that are blocking the Jenkins plugin BOM upgrade from Jenkins 2.522 to 2.523. Blocked pull request is: * jenkinsci/bom#5519 New interface was added in Jenkins core pull request: * jenkinsci/jenkins#10827 Add a script approval for jenkins.model.FullyNamed.getFullName() since the tests already have a script approval for hudson.model.Item.getFullName(). Keeps the tests consistent and allows the tests to pass with both older and newer Jenkins versions. Testing done: Confirmed that tests pass with Jenkins 2.523 and with default Jenkins version.
These correspond to methods
getName,getFullName,getFullDisplayNamethat did not have any common interface to call them, causing some inconvenient usages of switch statements or even reflection in CloudBees CI HA.See JENKINS-XXXXX.
Testing done
Proposed changelog entries
getName,getFullName,getFullDisplayNamecan now be referred through dedicated interfaces.Proposed changelog category
/label
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).