Add remaining missing verify/update targets into make#5111
Add remaining missing verify/update targets into make#5111SgtCoDFish wants to merge 7 commits intocert-manager:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SgtCoDFish The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| # a lesser evil than blocking our potential future upgrade to go 1.18 behind the release | ||
| # of k8s 1.24 | ||
| K8S_CODEGEN_VERSION=5915ef051dfa0658ffebb9af39679e52c31762bf | ||
| K8S_CODEGEN_VERSION=v0.24.0 |
There was a problem hiding this comment.
note: this is actually a benefit of the fact that I prioritised the website refresh in favour of this; k8s 1.24 came out in the meantime which removed the need for this big long comment and weird version 😁
| || command -v $(GO) >/dev/null || echo "$(GO) (or run 'make vendor-go')") \ | ||
| && (command -v $(CTR) >/dev/null || echo "$(CTR) (or set CTR to a docker-compatible tool)")) |
There was a problem hiding this comment.
note: tiny little change which makes it clearer that users can choose to vendor go and that they can use tools like podman instead of docker
|
/test pull-cert-manager-make-test |
|
/hold Our tests aren't calling |
|
@munnerz I think if we're gonna remove the bazel tests here I should roll the update-crds stuff into this PR as well as the existing stuff about update-codegen. I've got it written and I'd saved it for a separate PR but I'm thinking now we might as well remove these tests from bazel and get everything ready so we can remove the |
Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
|
/test pull-cert-manager-make-test |
c6c4b99 to
4d07c80
Compare
- runs "make update-codegen" - adds codegen verification to make tests - changes hack/(update|verify)-codegen.sh to just call make - removes bazel codegen test so it's not automatically run in CI Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
- includes a run of make update-crds which causes some trivial changes - (bonus) updates version of YQ to latest - makes hack/update-crds.sh just call make - makes hack/verify-crds.sh just call make - moves functionality of hack/verify-crds.sh to hack/check-crds.sh, using the makefile for generating alternative CRDs for comparison - removes the bazel test associated with CRDs Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
- hack/update-deps-licenses.sh becomes a muscle-memory-preserver - bazel test for deps-licenses is removed Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
these won't be added to make because they're quite tightly tied to bazel Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
since they're implemented in make, there's no reason to also have them run by bazel Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
|
/test pull-cert-manager-bazel-nocache |
|
/test all |
|
@SgtCoDFish: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. |
|
Just gonna close this because this piece of work is going to be a bigger thing I think |
Pull Request Motivation
Add remaining missing verify/update targets for codegen and CRDs into make (i.e. the things that we were keeping the
pull-cert-manager-bazelpresubmit around for).Also removes the related bazel targets entirely (but keeps the scripts which the bazel targets used to call, to preserve muscle memory)
NB: In standup on 2022-05-13 we agreed that
(verify|update)-depsdoesn't really fit naturally with the makefile build system, so it won't be ported.Kind
/kind feature
Release Note