search

jenkinsci / gogs-webhook-plugin

Jenkins Gogs Webhook
https://plugins.jenkins.io/gogs-webhook/
MIT License
79 stars 42 forks source link

Support pipeline jobs #1

Closed ssouris closed 7 years ago

ssouris commented 7 years ago

Hi there,

it's really great that your plugin became official. With this commit I am enabling the execution of pipeline jobs.

Please take a look and if you agree maybe add to your next release :)

Thanks and congrats on your plugin publishing.

sanderv32 commented 7 years ago

I will look at the changes tomorrow, and if everything is alright i will merge them into version 1.0.5.

xperimental commented 7 years ago

I also noticed yesterday that this plugin does not recognize "Pipeline" jobs. My fix just changes the use of AbstractProject to BuildableItem which is an interface also implemented by the WorkflowJob. This seems like a much simpler solution, but I do not know if this has any unintended consequences.

You can see my version in the "fix-job" branch of my fork. I didn't want to open another PR, but can do that if you think that approach is better.

sanderv32 commented 7 years ago

I will check your pull request and also the previous pull request if this is will not break standard functionality.

ehirsch commented 7 years ago

+1 for this one. I just spend some hours finding out that pipeline jobs are not supported yet. Anything I can do to get this merged?

ssouris commented 7 years ago

@ehirsch sorry to intervene here but if you are really in a hurry, you can always compile it yourself and import it into Jenkins.

sanderv32 commented 7 years ago

I want to make sure that also the current functionality doesn't break. It should not be that after the pull only pipeline builds will work and other users will suffer.

ssouris commented 7 years ago

@sanderv32 I tried on my machine with freestyle jobs and pipeline jobs and it worked :) So no pressure at all take your time and create a stable build. I am just giving @ehirsch an alternative until then. :)

ehirsch commented 7 years ago

@ssouris I am not that good in jenkins-fu. So your alternative is not an option. But that's no big deal. Now that I know about the limitation I can work around it… @sanderv32 take your time and thanks for your plugin.

bozaro commented 7 years ago

IMHO, fix by @xperimental looks like much better: https://github.com/jenkinsci/gogs-webhook-plugin/compare/master...xperimental:fix-job

ssouris commented 7 years ago

@bozaro and @all I agree with you it is very very simple. Nonetheless, @xperimental has not tested his solution. IMHO we should not push untested code and also it is not the maintainer's job to try an untested PR. :)

bozaro commented 7 years ago

I try fix https://github.com/jenkinsci/gogs-webhook-plugin/compare/master...xperimental:fix-job on my Jenkins instance and it looks like work.

xperimental commented 7 years ago

Nonetheless, @xperimental has not tested his solution.

Sorry for the misunderstanding, but my implementation is not untested. I have been running a build using my version since before posting the comment on this PR. The part with the "unintended consequences" should just highlight that I have not tested it for anything apart from what I use personally (which are "Free Style" and pipeline jobs).

sanderv32 commented 7 years ago

@ssouris I must admit that the pull request of @xperimental is at the moment the cleanest solution. Are you doing extra stuff which is needed?

ssouris commented 7 years ago

No @sanderv32 no extra stuff just borrowed code from gitlab plugin.

sanderv32 commented 7 years ago

Decided to merge request #2 because impact is minimal to the plugin.