Skip to content

Conversation

@jglick
Copy link
Member

@jglick jglick commented Jun 5, 2018

In favor of jenkinsci/apache-httpcomponents-client-4-api-plugin#9 as picked up by jenkinsci/workflow-basic-steps-plugin#60 & jenkinsci/copyartifact-plugin#104, the only places where this method had been called.

Changelog entry

  • RFE - Developer: Remove hudson.FilePath#copyFromRemotely() Beta API from the core

@jglick jglick added the on-hold This pull request depends on another event/release, and it cannot be merged right now label Jun 5, 2018
@jglick jglick requested review from carlossg and kuisathaverat June 5, 2018 22:52
@daniel-beck
Copy link
Member

Is this the first case of breaking updates due to Beta annotations?

@jglick
Copy link
Member Author

jglick commented Jun 5, 2018

Is this the first case of breaking updates due to Beta annotations?

Yes, though the user impact should be zero since the calls to copyFromRemotely are only made when encountering a VirtualFile implementing toExternalURL, which only exists in artifact-manager-s3, which has never been released to the UC.

@oleg-nenashev oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jun 6, 2018
@oleg-nenashev
Copy link
Member

Ready to merge once @jglick confirms it

@oleg-nenashev
Copy link
Member

@jglick please follow the PR template and create a changelog entry proposal. Although the API is not used elsewhere, IMHO it is important to make it changelogged since the API is in LTS.

Maybe we also want to create a follow-up patch to 2.107.x Stable branch and change annotation to@Restricted(DoNotUse.class) there

@jglick jglick removed the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Jun 6, 2018
@jglick
Copy link
Member Author

jglick commented Jun 6, 2018

All makes sense.

On hold until referenced PRs are merged.

@jglick
Copy link
Member Author

jglick commented Jun 8, 2018

jenkinsci/workflow-basic-steps-plugin#60 need not be merged first, since that is the same PR where this API was first used—has just been open that long. jenkinsci/copyartifact-plugin#104 ought to be merged and released first (implying a release of jenkinsci/apache-httpcomponents-client-4-api-plugin#9), since otherwise there could be an artifact-manager-s3 release which offers external URLs which is used on a Jenkins without copyFromRemotely but copyartifact 1.40 which tries to call it, leading to a linkage error. @ikedam is maintainer, no response so far, so TBD.

@jglick
Copy link
Member Author

jglick commented Jun 19, 2018

jenkinsci/copyartifact-plugin#104 has been merged but not yet released, so this is still on hold.

@jglick
Copy link
Member Author

jglick commented Jul 3, 2018

Trying to trigger a fresh build.

@jglick jglick closed this Jul 3, 2018
@jglick jglick reopened this Jul 3, 2018
@jglick jglick added ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback and removed on-hold This pull request depends on another event/release, and it cannot be merged right now labels Jul 3, 2018
@oleg-nenashev
Copy link
Member

Created https://issues.jenkins-ci.org/browse/JENKINS-52417 for it. IMHO tickets must be always required for Beta API removal. The change will need to be backported as discussed above

@oleg-nenashev oleg-nenashev changed the title Removing FilePath.copyFromRemotely [JENKINS-52417] - Removing FilePath.copyFromRemotely Jul 7, 2018
@oleg-nenashev oleg-nenashev merged commit 3a0c222 into jenkinsci:master Jul 7, 2018
@oleg-nenashev
Copy link
Member

Added the changelog entry

@jglick jglick deleted the network-JENKINS-50597 branch July 9, 2018 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants