-
-
Notifications
You must be signed in to change notification settings - Fork 98
Fleshing out backwards compatibility section of JEP-202 #98
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
Conversation
oleg-nenashev
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.
Thanks for doing that! Looks good. For obsolete API usages it may make sense to create a follow-up ticket to the core
|
|
||
| === Master-based file streaming | ||
|
|
||
| Some plugins using ``VirtualFile``s corresponding to build artifacts are still calling `open` |
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.
wow, didn't know about this syntax (double-single-quote)
| As seen in | ||
| link:https://ci.jenkins.io/job/Infra/job/deprecated-usage-in-plugins/job/master/lastSuccessfulBuild/artifact/output/usage-by-api.html#hudson_model_Run_getArtifactsDir__Ljava_io_File_[this report], | ||
| there are a number of plugins on the usual update center still calling `Run.getArtifactsDir()` and/or `Run.Artifact.getFile()`, | ||
| despite the fact that these methods were deprecated in Jenkins 1.531 in 2013 as part of |
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.
Maybe it makes sense to modify the API methods and to print WARNINGs if these APIs are used with custom ArtifactManager configurations. Then it can be somehow routed to telemetry in JEP-304
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.
Yes, was already suggested in this JEP, but does not do much good until JEP-304 is live I suppose.
| [cols="5,>",options="header",width="50%"] | ||
| |============================ | ||
| |Plugin|Installations | ||
| |link:https://plugins.jenkins.io/maven-plugin[Maven Integration]|124783 |
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.
Oh, should we rename it to die-hard-plugin? :D
| These include: | ||
|
|
||
| [cols="5,>",options="header",width="50%"] | ||
| |============================ |
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.
Would it make sense to create RFE tickets and reference them from JEP? maybe it's rather preferable to it in a Wiki page so that we could do it dynamically (like https://wiki.jenkins.io/display/JENKINS/Plugins+affected+by+fix+for+JEP-200 or https://wiki.jenkins.io/display/JENKINS/Plugins+affected+by+fix+for+SECURITY-170)
Then the warning message for old API could redirect people to that page
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.
I did not consider it worth bothering given the limited impact and the low installation counts. Would be possible if adding core logging, of course.
At any rate, that does not seem to have much bearing on the text of the JEP.
|
@oleg-nenashev You approved but then didn't merge. Should this be merged on not? |
|
I didn't merge it because there was no JEP Sponsor's approval at that point. |
As suggested by @oleg-nenashev in #96, enumerating plugins known to be using APIs which would clash with remote artifact storage. CC @carlossg