[JENKINS-64881] Bump dependencies to avoid PCT failure #42
Conversation
|
Optional reviewers: @rsandell @jtnord @olamy @bitwiseman @car-roll |
| <dependencyManagement> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>io.jenkins.tools.bom</groupId> | ||
| <artifactId>bom-2.222.x</artifactId> | ||
| <version>24</version> | ||
| <scope>import</scope> | ||
| <type>pom</type> | ||
| </dependency> | ||
| </dependencies> | ||
| </dependencyManagement> |
There was a problem hiding this comment.
given you only have 1 dependency here - do you really want this?
There was a problem hiding this comment.
Yes, regardless of number of dependencies, using the bom will avoid future PCT version mismatch failures.
There was a problem hiding this comment.
can you explain that - as the BOM is not a plugin dependency (nor is htmlunit) and not shipped with a war so how is it updated and how will it help here?
credentials-binding-pom-before.xml and credentials-binding-pom-after.xml show the same version in the PCT unit tests so some pointers so I can understand this would be very helpful
There was a problem hiding this comment.
Thought they were functionally different in some scenarios for enforcement of minimum upper bounds, but I was probably wrong. The last three commits in https://github.com/bitwiseman/github-branch-source-plugin/commits/dependencies/sample-1 show that the error is the same regardless of where the version is set.
Putting that aside, I would say that using the bom is considered best practice for all plugins we maintain. If someone comes along later and adds another dependency, the bom will already be there. Using the bom is more lines in this case, but it feels clearer.
Also, dependencyManagement is the best practice for ensuring specific version numbers for transitive dependencies (rather than depending on pom file and class loading ordering). I see the point that there is only one dependency, so this doesn't apply here, but again start with consistent best practices and stick with them.
Just my opinion.
See JENKINS-64881
This PR fixes a PCT failure (MsBuildBuilderTest#configRoundtrip) by updating HtmlUnit via updating the parent pom. We use the bom to better manage the dependencies.
In addition it improves the log printed in this method to know the root exception.
This also adds dependabot to the repository.