Code clean-up #185
Code clean-up #185
Conversation
|
For someone more experienced in the subject: Are adding |
In general, yes. You need to be careful with any classes that might be serialized. #190 should cover most of the diff here. Sorry about the PR review/merge backlog. |
|
I have dropped the JSR changes; conflicts resolved. |
| dstDir.child(path).copyFrom(in); | ||
| } finally { | ||
| IOUtils.closeQuietly(in); |
There was a problem hiding this comment.
There are no unused imports left or do you mean reordering / grouping?
There was a problem hiding this comment.
I did not check, I just guessed that IOUtils became unused.
src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jenkinsci/plugins/workflow/ArtifactManagerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jenkinsci/plugins/workflow/flow/DurabilityBasicsTest.java
Outdated
Show resolved
Hide resolved
|
No problem. Thanks for the suggestions. 👍 |
| @@ -109,7 +109,7 @@ public FlowDurabilityHint getDurabilityHint() { | |||
| /** | |||
| * Should be called by the flow owner after it is deserialized. | |||
| */ | |||
| public /*abstract*/ void onLoad(FlowExecutionOwner owner) throws IOException { | |||
There was a problem hiding this comment.
Changes to throws clauses like this break source compatibility with CpsFlowExecution in https://github.com/jenkinsci/workflow-cps-plugin and need to be reverted:
-------------------------------------------------------------
COMPILATION ERROR :
-------------------------------------------------------------
workflow-cps-plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java:[721,17] onLoad(org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner) in org.jenkinsci.plugins.workflow.cps.CpsFlowExecution cannot override onLoad(org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner) in org.jenkinsci.plugins.workflow.flow.FlowExecution
overridden method does not throw java.io.IOException
1 error
-------------------------------------------------------------
There was a problem hiding this comment.
Indeed this is wrong and should be reverted.
There was a problem hiding this comment.
@dwnusbaum if you approve #201 I can cut a release to fix this.
Some code clean-up and removal of JSR 305.