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

Run button parameterized builds #56

Merged

Conversation

jeffastorey
Copy link

Fixes issues relating to the run button for parameterized builds
https://issues.jenkins-ci.org/browse/JENKINS-20841
https://issues.jenkins-ci.org/browse/JENKINS-25427

Also switched to using app.rootUrl for the fillDialog method as recommended by the jelly guidelines (https://wiki.jenkins-ci.org/display/JENKINS/Basic+guide+to+Jelly+usage+in+Jenkins#BasicguidetoJellyusageinJenkins-PredefinedURLs). Newer versions of Jenkins changed rootUrl to be /jenkins and when the extra / was added by fillDialog, the URL did not work. Jenkins will automatically add the trailing slash to the app.rootUrl

@jeffastorey
Copy link
Author

I didn't explicitly run checkstyle and the build failed. I will update.

@jeffastorey
Copy link
Author

Checkstyle issues fixed, apologies for that.

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@roboll
Copy link

roboll commented Nov 9, 2014

this is a killer for me.. would love to see this merged and a new release cut

@KostyaSha
Copy link
Member

looks ok for me

@KostyaSha
Copy link
Member

Does it also fix #57 case ?

@jeffastorey
Copy link
Author

Yes, it should.
On Nov 17, 2014 6:53 PM, "Kanstantsin Shautsou" notifications@github.com
wrote:

Does it also fix #57
#57 case ?


Reply to this email directly or view it on GitHub
#56 (comment)
.

@christ66
Copy link
Member

@jeffastorey I promise @KostyaSha I'll do a review + test later tonight. Quick glance at it though LGTM.

@tompson
Copy link

tompson commented Nov 26, 2014

Please merge this and provide a new version, really missing the possibility to define the parameter for the first job

@KostyaSha
Copy link
Member

@tompson
Copy link

tompson commented Nov 26, 2014

@KostyaSha installed the snapshot, tested it and it works for me as expected

@KostyaSha
Copy link
Member

Thanks, i will ping @christ66, then merge and release.

KostyaSha added a commit that referenced this pull request Nov 26, 2014
@KostyaSha KostyaSha merged commit a89e7e4 into jenkinsci:master Nov 26, 2014
@KostyaSha
Copy link
Member

@tompson please test it using Folders plugin with jobs in folder.

@KostyaSha
Copy link
Member

I remembered! @christ66 wanted to test Folders support. We need test it before doing release.

@jeffastorey
Copy link
Author

It should work with folders. I tested with and without. But a double
check on that would be good.
On Nov 26, 2014 12:44 PM, "Kanstantsin Shautsou" notifications@github.com
wrote:

I remembered! @christ66 https://github.com/christ66 wanted to test
Folders support. We need test it before doing release.


Reply to this email directly or view it on GitHub
#56 (comment)
.

@KostyaSha
Copy link
Member

tested, works.

@KostyaSha
Copy link
Member

released as 1.4.5, will appear on mirrors in next 24h. Atm you can update manually using hpi and plugin manager http://repo.jenkins-ci.org/releases/org/jenkins-ci/plugins/build-pipeline-plugin/1.4.5/

@@ -63,7 +63,7 @@
{{#unless project.disabled}}
<div class="status-bar" id="status-bar-{{id}}">
{{#if build.isBuilding}}
<div class="pointer" onclick="buildPipeline.fillDialog('${rootURL}/{{build.url}}console', 'Console output for {{project.name}} #{{build.number}}')">
<div class="pointer" onclick="buildPipeline.fillDialog('${app.rootUrl}{{build.url}}console', 'Console output for {{project.name}} #{{build.number}}')">
Copy link
Member

Choose a reason for hiding this comment

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

If you are not ultimately inside <l:layout>, you can also use

<j:new var="h" className="hudson.Functions"/>
${h.initPageVariables(context)}

to initialize standard variables such as rootURL.

Copy link
Member

Choose a reason for hiding this comment

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

Is it documented somewhere? Is there an analogue to Groovy?

Copy link
Member

Choose a reason for hiding this comment

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

Normally you do not think about this because normally you would be inside l:layout which already sets this up for you.

@lonefreak
Copy link

I'm still facing problems to start pipelines with parameterised jobs. I'm using 1.4.8 and Jenkins 1.609.1

Following this (https://issues.jenkins-ci.org/browse/JENKINS-25427?focusedCommentId=237769&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-237769) I managed to solve the problem.

@jeffastorey would you consider changing the current version o fancybox used in the plugin? I changed it to the patched version (https://github.com/prdolmos/fancybox-1.3.4-patched-for-jquery-1.9) above and to official version 2.1.5 (http://fancyapps.com/fancybox/). Worked fine in both cases.

@jeffastorey
Copy link
Author

Can you submit a pull request for it? Ir sounds like you've already got it worked out. I'm not the maintainer though so I won't be the one to approve it.

@lonefreak
Copy link

Will submit it. I was just asking because the ticket is resolved, but I
found that some people (including myself) is still having trouble.

Tks
On Sep 30, 2015 2:51 PM, "Jeff Storey" notifications@github.com wrote:

Can you submit a pull request for it? Ir sounds like you've already got it
worked out. I'm not the maintainer though so I won't be the one to approve
it.


Reply to this email directly or view it on GitHub
#56 (comment)
.

lonefreak pushed a commit to lonefreak/build-pipeline-plugin that referenced this pull request Sep 30, 2015
In its current version 1.4.8, the plugin is unable to start a pipeline
which the first job is pamateterised. It was possible on version 1.4.3
and according to jenkinsci#56
t should be possible again, but I only managed to run pipelines changing
the version o Fancybox to 1.3.4_patch as recommended here:

https://issues.jenkins-ci.org/browse/JENKINS-25427
@anentropic
Copy link

@lonefreak did you make a PR?

I am also suffering this problem and would love to see it fixed

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.

10 participants