Set Maven build requirements and some project POM metadata #267
Conversation
ches
requested review from
davidheryanto,
pradithya,
thirteen37,
tims,
woop and
zhilingc
as code owners
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ches If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @ches. Thanks for your PR. I'm waiting for a gojek member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Thanks @ches This definitely seems like something we want our tooling to do for us, especially if we want to accept contributions from the community. So thanks for adding it. We are simultaneously shifting our focus to updating our automated testing as well. Once that is done we will be able to guard ourselves from bad builds when new PRs are submitted. |
|
On a related note, we are still pushing code directly to this branch at the moment. Once the release is out we will all revert to using PRs for any new code submissions. |
Urge the use of <dependencyManagement> by disallowing duplicate dependency declarations. See #262. Making final releases with SNAPSHOT dependencies in use is a really bad idea that can happen by accidental oversight. Enforcer helps ensure it doesn't. For build reproducibility it's good to specify what Maven versions are supported for the project's build to work. 3.0.5 is the minimum required version by all our plugins currently, according to: $ mvn versions:display-plugin-updates
|
@ches: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
A goal of #262 was to standardize versions of dependencies used across modules in the project. If everyone is on board with that, then we want to avoid re-declaring versions outside of
<dependencyManagement>.If you're not used to
<dependencyManagement>it may help to think of it this way: it defines dependencies, while<dependency>references in modules "instantiate" them.Maven's Enforcer plugin provides a rule that detects duplicate dependency definitions and fails the build early. This adds a bit of friction nudging developers to understand this way of working. My feeling is this friction is more than worth the sum frustration of dependency hell that it can relieve over time.
I'll have some additions to
CONTRIBUTING.mdcoming sooner or later, if you think this warrants a note there I can make that sooner.(Enforcer has another useful
dependencyConvergencerule, but it's going to take a good bit of work to determine if all the divergences it currently reports can be safely resolved, and integration tests will help).The PR enables a couple of other build sanity checks with rationale in the commit message, and some project metadata that you may want to tweak but I started as placeholders at least.