[JENKINS-43052] Warn if a deprecated extension is used in Pipeline#3905
Conversation
|
@Nimbusprogrammer you need to resolve the spotbugs failure before I'm willing to review the proposed changes. Having spotbugs warnings says that you need to follow the instructions in the contributing guide and run Please close the preceding pull request if you are replacing it with this one. |
|
@MarkEWaite |
MarkEWaite
left a comment
There was a problem hiding this comment.
I've not completed the review, but some initial comments for you to consider and possibly apply in other areas of the pull request.
src/main/java/hudson/plugins/git/extensions/impl/PreBuildMerge.java
Outdated
Show resolved
Hide resolved
src/test/java/hudson/plugins/git/extensions/impl/PreBuildMergeWorkflowTest.java
Outdated
Show resolved
Hide resolved
src/test/java/hudson/plugins/git/extensions/impl/PreBuildMergeWorkflowTest.java
Outdated
Show resolved
Hide resolved
src/main/java/hudson/plugins/git/extensions/GitSCMExtension.java
Outdated
Show resolved
Hide resolved
src/main/java/hudson/plugins/git/extensions/impl/PreBuildMerge.java
Outdated
Show resolved
Hide resolved
|
@MarkEWaite I also made a small adjustment in the tests to normalize the repository path used in the Pipeline definitions so they behave consistently across platforms. All checks are now passing. Please let me know if you’d like any further changes. |
Consistent coding pattern and avoids the need for a variable to decide if the message has been displayed. Messages will be displayed each checkout. Still needs test for message exclusion deprecation and for requires workspace polling deprecation.
|
Thanks again for the pull request. While reviewing with the debugger, I saw that there were two cases that were not covered by tests. I've added tests to cover those cases. I also realized that the deprecation messages could be simplified by placing each of them in the I also reduced the differences to the master branch so that the comparison would be smaller and show only the changes. I also changed the Windows escaping of the path to the sample repository to use the Windows path separator with escaping as needed. That assures were testing Windows paths in the "Windows way". I still need to perform interactive testing to confirm the end user experience. |
Includes pull request: * jenkinsci/git-plugin#3905
|
@MarkEWaite Thanks for the update and for the detailed follow-up. I appreciate you adding coverage for the additional cases and simplifying the implementation. Please let me know if there’s anything you’d like me to adjust or help verify. |
MarkEWaite
left a comment
There was a problem hiding this comment.
Thanks! I've completed interactive testing. I added test jobs to my interactive test environment so that it will continue to be easy to check.
Implements Pipeline-only DEPRECATED warnings when deprecated GitSCM extensions are used, as described in JENKINS-43052.
The warning is written only to Pipeline build logs and does not alter behavior. Freestyle jobs are unaffected. Where applicable, the warning suggests the preferred Pipeline-native alternative.
Testing done
Submitter checklist
Jira: https://issues.jenkins.io/browse/JENKINS-43052