{"action":"created","issue":{"url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/12","repository_url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin","labels_url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/12/labels{/name}","comments_url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/12/comments","events_url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/12/events","html_url":"https://github.com/jenkinsci/nodelabelparameter-plugin/pull/12","id":178513044,"number":12,"title":"Use value as the parameter element, instead of labels","user":{"login":"kinow","id":304786,"avatar_url":"https://avatars.githubusercontent.com/u/304786?v=3","gravatar_id":"","url":"https://api.github.com/users/kinow","html_url":"https://github.com/kinow","followers_url":"https://api.github.com/users/kinow/followers","following_url":"https://api.github.com/users/kinow/following{/other_user}","gists_url":"https://api.github.com/users/kinow/gists{/gist_id}","starred_url":"https://api.github.com/users/kinow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kinow/subscriptions","organizations_url":"https://api.github.com/users/kinow/orgs","repos_url":"https://api.github.com/users/kinow/repos","events_url":"https://api.github.com/users/kinow/events{/privacy}","received_events_url":"https://api.github.com/users/kinow/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2016-09-22T03:25:22Z","updated_at":"2016-09-22T10:22:38Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/pulls/12","html_url":"https://github.com/jenkinsci/nodelabelparameter-plugin/pull/12","diff_url":"https://github.com/jenkinsci/nodelabelparameter-plugin/pull/12.diff","patch_url":"https://github.com/jenkinsci/nodelabelparameter-plugin/pull/12.patch"},"body":"Hi, \r\n\r\nIn order to support using NodeLabel parameters in Active Choices plug-in, we can either try to add special logic to support NodeLabel parameters, or try to make the plug-in behave similar to other parameters.\r\n\r\nA choice parameter in Jenkins creates the following HTML.\r\n\r\n```\r\n<div name=\"parameter\">\r\n  <input name=\"name\" value=\"CCC\" type=\"hidden\">\r\n  <select name=\"value\"><option value=\"1\">1</option><option value=\"2\">2</option><option value=\"3\">3</option><option value=\"4\">4</option></select></div>\r\n```\r\n\r\nBut the NodeLabel parameter creates the following HTML.\r\n\r\n```\r\n<div description=\"\" name=\"parameter\">\r\n  <input name=\"name\" value=\"NODES_PARAM\" type=\"hidden\">\r\n  <select name=\"labels\"><option value=\"master\">master</option><option value=\"manga\">manga</option><option value=\"test1\">test1</option></select></div>\r\n```\r\n\r\nThe main difference is the select name, which instead of \"value\" is \"labels\" for the NodeLabel parameter. It gets interpreted correctly in Jenkins later, when the job is executed. But since active choices inspects HTML DOM to retrieve values, it is not compatible with the NodeLabel plug-in.\r\n\r\nI've quickly tested it, and the Node parameter was correctly passed to the build when using value instead of labels. But I also noticed JENKINS-28374, and parts of the code like this https://github.com/jenkinsci/nodelabelparameter-plugin/blob/db037f2e2586e545c57399ce32e2b7436772b8e3/src/main/java/org/jvnet/jenkins/plugins/nodelabelparameter/NodeParameterDefinition.java#L228\r\n\r\nSo not entirely sure it wouldn't cause a regression in the plug-in. What do you think @imod?\r\n\r\nCheers\r\nBruno"},"comment":{"url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/comments/248863774","html_url":"https://github.com/jenkinsci/nodelabelparameter-plugin/pull/12#issuecomment-248863774","issue_url":"https://api.github.com/repos/jenkinsci/nodelabelparameter-plugin/issues/12","id":248863774,"user":{"login":"kinow","id":304786,"avatar_url":"https://avatars.githubusercontent.com/u/304786?v=3","gravatar_id":"","url":"https://api.github.com/users/kinow","html_url":"https://github.com/kinow","followers_url":"https://api.github.com/users/kinow/followers","following_url":"https://api.github.com/users/kinow/following{/other_user}","gists_url":"https://api.github.com/users/kinow/gists{/gist_id}","starred_url":"https://api.github.com/users/kinow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kinow/subscriptions","organizations_url":"https://api.github.com/users/kinow/orgs","repos_url":"https://api.github.com/users/kinow/repos","events_url":"https://api.github.com/users/kinow/events{/privacy}","received_events_url":"https://api.github.com/users/kinow/received_events","type":"User","site_admin":false},"created_at":"2016-09-22T10:22:38Z","updated_at":"2016-09-22T10:22:38Z","body":">it would probably be a good idea to have a test case that verifies we still support both, labels and value.\r\n\r\nI agree @imod. But when I was writing the pull request, and thought how to write some test for that, I couldn't find a simple way of doing so.\r\n\r\nAs there's no more labels parameter. The curl calls are already using 'value'; and now the Jelly view is also using 'value' instead of 'labels'.\r\n\r\nWhat I did before submitting the pull request was create a job before the change, using nodelabels. Test, and confirm it works. Then update the Jelly files, refresh the pages, and confirm it's still working. Then ran `mvn clean test install`, and there were no errors.\r\n\r\nI'm not sure how to add some unit tests to make sure it won't add any regression. I was assuming having existing tests passing, plus running some manual tests, would provide a good initial base for this change. What do you think @imod? If you have any suggestions I can try it during this weekend.\r\n\r\nThanks\r\nBruno"}}
{"actor":{"display_login":"kinow"}}