-
-
Notifications
You must be signed in to change notification settings - Fork 66
Facelift plugin to make it testable in PCT (for JEP-200) #95
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
base: master
Are you sure you want to change the base?
Conversation
|
@reviewbybees @jglick |
Wadeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐝 LGTM
tomasbjerre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
But, as I've mentioned before, I don't see the point of maintaining this plugin without a complete rewrite. And I have such a rewrite ready since november 2016. #88
|
@tomasbjerre IMHO it makes sense to ship the rewrite as a major 1.0 release. I tried the current alpha version in January, and it seems to work well for me. CC @uhafner |
|
I think we already discussed this in #88. The conclusion was (from my point of view): the rewrite of the violations plug-in on top of analysis-core makes no sense since we then have a warnings plug-in Vol. 2 that has a subset of the features and parsers of the warnings plugin Vol. 1. For me, it makes more sense to discontinue support for the violations plugin and integrate any missing features into the warnings plug-in (are there any?). In the meantime the warnings plugin and analysis-core has been undergoing a total rewrite in order to allow an integration of parsers of any external library, see https://issues.jenkins-ci.org/browse/JENKINS-40439 for details. What still needs to be done: integrate the parsers of Tomas' violations-lib into the warnings plug-in. I hope to provide a PR after my ski holidays. |
|
Yeah, I should have read threads first. |
|
Yes, that will be part of my PR. The parsers of the violations-lib are not yet serialisable at all making it impossible to use them on agents. (I'll run the tests using Jenkins 2.102+ too see if everything is running smooth) |
|
Any issues with the lib should not be discussed here... but just for the record. There has been no issues reported about problems running on agents. I also tried that now, using this plugin, and I see no problems. Edit: also tried with an agent and 2.102 now. Also no problems. |
| @@ -0,0 +1,2 @@ | |||
| // Build the plugin using https://github.com/jenkins-infra/pipeline-library | |||
| buildPlugin(jenkinsVersions: [null, '2.104']) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2.107 + jenkinsci/plugin-pom#95
|
Looks like Violations plugin stopped to work on the recent stable Jenkins build Jobs that have Violations step simply stuck and never finish. Worst is that I can't even go to the There are no problems with jobs that doesn't use Violations plugin. Any ideas how to proceed? I tried to build and install this branch by @oleg-nenashev without any luck. Looks like I also can't revert to older version of Jenkins as this latest |
|
Thanks @oleg-nenashev . I'm just wondering is it realistic to expect a fix soon or my best bet is to try to downgrade? |
|
@sauliusgrigaitis Violations plugin is deprecated, I would not anticipate a fix soon. Generally it depends on the #88 release I'd guess |
|
Thanks @oleg-nenashev . I'm just wondering should #88 solve the issue or it doesn't address it? |
|
No, this PR won't solve the issue. It's not even related to it |
|
Noting that any cleanup/modernization effort should fix violations-plugin/src/main/java/hudson/plugins/violations/ViolationsCollector.java Lines 253 to 256 in cab7103
(This unreleased addition might actually introduce a security vulnerability in older releases of Jenkins) |
Uh oh!
There was an error while loading. Please reload this page.