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

(Bug Fixed!)JENKINS-28709 Join Plugin ignores dependencies wrapped with flexible-… #10

Merged
merged 3 commits into from May 31, 2016

Conversation

Edgar0119
Copy link

Fix bug JENKINS-28709
Now it can handle "hudson.plugins.parameterizedtrigger.BuildTrigger" in Flexible publisher!

…publish

(https://issues.jenkins-ci.org/browse/JENKINS-28709)

Now it can handle "hudson.plugins.parameterizedtrigger.BuildTrigger"  in Flexible publisher!
@mdonohue
Copy link
Member

This is much more readable - thank you for fixing the whitespace.

I have two more suggestions here:

  1. The Flexible Publisher should be an optional dependency
  2. Can you explain how the code behaves if the flexible publisher plugin is not installed?

@Edgar0119
Copy link
Author

Hello,
Flexible Publisher plugin cannot be an optional dependency,
It just like the join-plugin has dependency on parameterized-trigger, like this (original code)line https://github.com/jenkinsci/join-plugin/pull/10/files#diff-e6e42dd5529efb392db38cf6e0030c29R265
The join-plugin was support parameterized-trigger plugin, so that join-plugin dependency on parameterized-trigger,and if the join-plugin work without parameterized-trigger, that will be crash.
Similarly, the join-plugin has dependency on Flexible Publisher plugin.

@mdonohue
Copy link
Member

I just tried this with a fresh jenkins 1.600 install, no parameterized trigger, no flexible publisher - the published 1.19 version of the join plugin works fine. I can setup a simple join scheme and it runs as expected.

With the current pull request, the join plugin won't even install. It is rejected because the 'flexible-publisher' dependency isn't marked as optional.

Please make it optional, and test on a jenkins install without the flexible publisher. The join plugin should continue to operate as it did before. If you look carefully at how the parameterized trigger code works, it uses runtime lookups, to make sure the presence of the plugin is not required.

@Edgar0119
Copy link
Author

I'm sorry , I misunderstood what you mean , but now I understand.

I will let flexible publisher be an optional dependency, and I'll ensure join plugin work correctly without flexible publisher installed.

Thank you for your patience and suggestions .

@mdonohue
Copy link
Member

Thanks for the fixes!

@mdonohue mdonohue merged commit 7ff6743 into jenkinsci:master May 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants