v0.3 backport: Fail Spotless check before tests #516
Conversation
ches
requested review from
davidheryanto,
pradithya,
thirteen37,
woop and
zhilingc
as code owners
|
/assign @thirteen37 |
|
The test failures for Java say: Is there configuration somewhere that maybe has the old branches running on old image versions? |
Never mind, @davidheryanto did this already in #496 🙏 |
I guess it's perhaps these: The e2e test scripts have been updated to require Debian Stretch, but |
I think its actually based on Correct me if I am wrong @davidheryanto, but you are using this So in theory we should just backport the removal of the stretch requirement or change it to buster? |
|
Oh I think Docker image I think it should be generally safe to remove the check for Debian version, I put it last time just for completeness. https://github.com/gojek/feast/blob/v0.3-branch/.prow/scripts/test-end-to-end.sh#L6 I will created a PR to v0.3-branch to remove this check |
By default, the spotless Maven plugin binds its check goal to the verify phase (late in the lifecycle, after integration tests). Because we currently only run `mvn test` for CI, it doesn't proceed as far as verify so missed formatting is not caught by CI. This binds the check to an earlier phase, in between test-compile and test, so that it will fail before `mvn test` but not disrupt your dev workflow of compiling main and test sources as you work. This strikes a good compromise on failing fast for code standards without being _too_ nagging. For the complete lifecycle reference, see: https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html
Hopefully for last time as a bulk operation, after 6363540.
ches
force-pushed
the
v0.3-backport-487-spotless-early
branch
from
6a0de15
to
67e43d2
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ches, davidheryanto 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 |
What this PR does / why we need it:
Backports #487 to
v0.3-branch—as we are internally developing on this branch for the time being, we're trying to maintain formatting for the sake of easing reviews as intended, and also in hopes that it will ease cherry picks for back- and possible forward-porting since #487 keeps consistency.I'll send one of these for
v0.4-branchtoo, as we are edging closer to catching up to v0.4 🎉Does this PR introduce a user-facing change?: