[FIXED JENKINS-35410] Improve User Experience #3
Conversation
|
I agree with a separate plugin. Actually, the path handling logic has been already implemented in jenkinsci/promoted-builds-plugin#62 , to it just needs some decoupling and documentation. |
|
|
||
| public FormValidation doCheckProject(@QueryParameter String project) { | ||
| if (project.startsWith("/")) { | ||
| return FormValidation.warning(Messages.BatchTaskInvoker_ForwardSlash()); |
There was a problem hiding this comment.
Maybe it should be an error
| @@ -139,6 +141,27 @@ public ListBoxModel doFillTaskItems(@QueryParameter String project, @AncestorInP | |||
| } | |||
| return r; | |||
| } | |||
|
|
|||
| public FormValidation doCheckProject(@QueryParameter String project) { | |||
There was a problem hiding this comment.
Please add Restricted(NoExternalUse) to such new validation hooks.
It prevents compatibility issues when you upgrade the stuff
|
Now, I need to delete .idea/libraries/Maven__xpp3_xpp3_min_1_1_4c.xml |
|
Done it! I think this should be ready to go! |
| The <strong>Full project name</strong> | ||
|
|
||
| <p> | ||
| For jobs inside a folder it is always a path like <strong>foo/bar</strong> |
There was a problem hiding this comment.
NIT: maybe makes sense to reference other use-cases: E.g. multibranch projects, etc.
|
🐝 with minor comments |
|
That would at least make the configuration screen consistent (and documented) with the expected values in the implementation. But are you sure it's so much hard to fix the real issue rather than enforcing that wrong behavior which prohibits the use of relative paths (thus the folder copy) and makes it mandatory to use an absolute path written like a relative path (missing leading slash)?! As far as I can see, the issue comes from https://github.com/jenkinsci/batch-task-plugin/blob/master/src/main/java/hudson/plugins/batch_task/BatchTaskInvoker.java#L78 which uses a wrong context (root instead of the current folder). In contrary, your addition is aligned on the wrong call line 78 using Could you please give a try at fixing the line 78, providing it the current context? I'm not very familiar with the plugin structure and what variable is available where :/ Finally, note that even the API of |
fbelzunc
changed the title
|
IIRC currently, it is getting the Item using:
I don't think to change this is just a line of code. Right now it is asking for Project Full Name, to support absolute and relative paths is a different story and will need test to check that works as expected and don't break current set-up for people already using this plugin. Feel free to submit a PR adding the relative/absolute paths. It will be very welcome. |
fbelzunc
changed the title
For the moment I prefer not fully support the / because it will be a hack on the plugin, this should be addressed on jenkins-core or in a separate plugin.
https://issues.jenkins-ci.org/browse/JENKINS-35188
@reviewbybees