-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
[JENKINS-51483] Remove erroneous entry in split-plugin-cycles.txt #3454
Conversation
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.
🐝 thx for the context / information
@jglick Would be great if you could confirm whether this will cause any serious issues. |
First of all, the dependency is in the other direction. Second, this file is listing dependencies which are implied by the split list, not defined in the POM, but which are not in fact real—would quietly disappear if the plugin on the left side were simply rebuilt against a newer core version. |
@jglick Thanks for looking!
In this case, the implied dependency was real (at least when running tests), because apache-httpcomponents-client-4-api had some tests that used JDKInstaller. Now, I realize that the (thankfully removed) dependency on apache-httpcomponents-client-4-api from jdk-tool was totally incorrect because My intention with this PR is to restore the implied dependency. I will update the description and changelog. |
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.
To the extent I understand.
Test failure is unrelated, INFRA issue |
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.
👍 since I am the author of those tests which got removed (and this is fine)
See JENKINS-51483.
This entry should have been removed before #3301 was merged.
Here is the jdk-tool:1.0 pom that shows that it does not depend on apache-httpcomponents-client-4-api: https://github.com/jenkinsci/jdk-tool-plugin/blob/jdk-tool-1.0/pom.xml.EDIT: jdk-tool is the wrong place to justify this. The actual justification is that apache-httpcomponents-client-4-api used JDKInstaller in some tests and the entry should never have been added.Proposed changelog entries
Submitter checklist
* Use the
Internal:
prefix if the change has no user-visible impact (API, test frameworks, etc.)Desired reviewers
@reviewbybees