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

Fix for NodeLabel plugin #30

Closed
wants to merge 1 commit into from
Closed

Fix for NodeLabel plugin #30

wants to merge 1 commit into from

Conversation

rsandell
Copy link
Member

Sent to me via mail

I'm not familiar with GitHub, but when using Rebuild plugin with NodeLabel plugin, I have been facing some issues.

I have succeeded to fix it for my own jobs, and I give you my code if you want to deliver it on GitHub, you're welcome.

@oleg-nenashev
Copy link
Member

It would be preferable to extend/use RebuildParameterProvider for the case, but the fix stills valid

Constructor ctor = oldValue.getClass().getConstructor(String.class, String.class);
return (ParameterValue)ctor.newInstance(new Object[]{oldValue.getName(), newValue});
} catch (Exception e) {
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe needs logging

@orgads
Copy link

orgads commented Dec 6, 2016

This should solve JENKINS-38635. Please merge it.

@@ -67,6 +68,7 @@
public class RebuildAction implements Action {

private static final String SVN_TAG_PARAM_CLASS = "hudson.scm.listtagsparameter.ListSubversionTagsParameterValue";
private static final String NODE_LABEL_PARAM_CLASS = "org.jvnet.jenkins.plugins.nodelabelparameter.LabelParameterValue";
Copy link
Member

Choose a reason for hiding this comment

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

there is also org.jvnet.jenkins.plugins.nodelabelparameter.NodeParameterValue, which extends org.jvnet.jenkins.plugins.nodelabelparameter.LabelParameterValue

@jdsn
Copy link

jdsn commented Dec 9, 2016

👍 please merge it, it would make rebuild plugin useful for us again

@orgads
Copy link

orgads commented Dec 9, 2016

Actually I tried it and it didn't work. Did anyone else verify it?

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

🐛 since the fix is incomplete (see the comment from @imod). But I'm looking forward to get it fixed and released

@jdsn
Copy link

jdsn commented Dec 9, 2016

Oh, too bad.

@rsandell can you please have a look at this PR and change it so it can be merged finally?

@ejpenney
Copy link
Contributor

ejpenney commented Jan 31, 2017

Not sure if this helps, but I noticed that changing this line in NodeParameterValue.jelly:

<f:textbox name="labels" value="${it.label}" />
and removing the "s" in "labels":
<f:textbox name="label" value="${it.label}" />
Fixed the issue for me. Rebuilds of jobs with NodeParameterValues seems to work now, but I'm sure this breaks some other use case not relevant to me.

@rsandell
Copy link
Member Author

rsandell commented Aug 4, 2017

Well this actually not my change as it says in the description. I only uploaded the patch for the one who sent it to me and gave him the link hoping it wasn't a drive by contribution :'(

@GLundh
Copy link
Member

GLundh commented Oct 19, 2017

I have merged #45 which seems to work fine for me. Any reason to keep this open? @jdsn : Do you want to verify that the issue is fixed on the master branch?

@GLundh
Copy link
Member

GLundh commented Oct 27, 2017

No response for 8 days. Closing this one.

@GLundh GLundh closed this Oct 27, 2017
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.

8 participants