Support for Item categorization in Jenkins 2 #9
Conversation
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
|
As part of this PR I'd like to remove the link to the documentation in the description. |
| * @return A string it represents a URL pattern to get the Item icon in different sizes. | ||
| */ | ||
| public String getIconFilePathPattern() { | ||
| return (Jenkins.RESOURCE_PATH + "/plugin/external-monitor-job/images/:size/externaljob.png").replaceFirst("^/", ""); |
There was a problem hiding this comment.
@jglick you prefer this way, right?
There was a problem hiding this comment.
Yes, so long as the API contract remains that the implementation must return a context-relative path.
(I briefly considered whether it would be appropriate for the caller to automatically prepend RESOURCE_PATH but this seems a little risky, and inconsistent with other Jenkins APIs defining image paths.)
There was a problem hiding this comment.
@jglick If you agree, I prefer to use the original format, used in other plugins:
public String getIconFilePathPattern() {
return "plugin/external-monitor-job/images/:size/externaljob.png";
}
In my opinion, more flexible.
There was a problem hiding this comment.
more flexible
How so?
Not a big deal either way, just means you will lose icon caching for repeated page loads.
|
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
|
🐝 |
| * @return A string it represents a ItemCategory identifier. | ||
| */ | ||
| public String getCategoryId() { | ||
| return "standaloneprojects"; |
There was a problem hiding this comment.
Why it is this string and not any other should be documented somewhere IMO.
There was a problem hiding this comment.
I would expect it to be available via Enum or a static variable from the Jenkins core 2.0. IMHO there should be a TODO for it
There was a problem hiding this comment.
When this plugin upgrades his baseline, he will able to use something like that: StandaloneProjectsCategory.ID. By now, if we want that these plugins use ItemCategorization feature (provided by Jenkins 2.0) and additionally are compatible with Jenkins 1.x, this was the best option.
|
🐝 |
|
🐜 for the logo. It's a bit strange. It may also confuse people if somebody creates the "Item Type Icon Column" for list views, because this logo contains a blue ball similar to the status indicator 🐝 for the other stuff |
|
@oleg-nenashev The icon was designed by @gusreiber not me. |
|
@reviewbybees done |
|
This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging. |
This PR belongs to this JENKINS-31162 issue and allows to categorize the External Monitor Jobs in a right way.
@reviewbybees