Skip to content

Presubmits: enable make test-ci and make e2e-ci by default#648

Merged
jetstack-bot merged 1 commit intocert-manager:masterfrom
maelvls:apk-add-grep-tar
Mar 17, 2022
Merged

Presubmits: enable make test-ci and make e2e-ci by default#648
jetstack-bot merged 1 commit intocert-manager:masterfrom
maelvls:apk-add-grep-tar

Conversation

@maelvls
Copy link
Copy Markdown
Member

@maelvls maelvls commented Mar 17, 2022 •
edited
Loading

This PR is part of the effort 'moving away from Bazel' tracked in cert-manager/cert-manager#4712

This PR follows up the PR cert-manager/cert-manager#4914 that just got merged. As mentioned in the TODO,

# TODO: set to "always_run: true" and "optional: false" as soon as
# https://github.com/cert-manager/cert-manager/pull/4914 is merged.

This PR does exactly that.

Note that every open PR on cert-manager/cert-manager will have to get rebased in order to pass the two new tests pull-cert-manager-make-test and pull-cert-manager-make-e2e-v1-23. I will leave a message on each of the 33 open PR.

@munnerz made me aware of the fact that Prow runs the jobs on the merge commit, which means there is no need to rebase. But each PR (may?) have to get the required test manually triggered with the following:

/test pull-cert-manager-make-test
/test pull-cert-manager-make-e2e-v1-23

If that's the case, I will trigger these tests on each individual PR.

@maelvls
presubmits: enable make test-ci and make e2e-ci by default
6069756 
Signed-off-by: Maël Valais <mael@vls.dev>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. area/cert-manager Indicates a PR related to cert-manager approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 17, 2022
@maelvls maelvls changed the title presubmits: enable make test-ci and make e2e-ci by default Presubmits: enable make test-ci and make e2e-ci by default Mar 17, 2022
@maelvls maelvls requested a review from SgtCoDFish March 17, 2022 09:11
This was referenced Mar 17, 2022
Merged
Merged
Merged
Merged
Open
Closed
Merged
Closed
Merged
Merged
Closed
Merged
Merged
Merged
Closed
Closed
SgtCoDFish
SgtCoDFish approved these changes Mar 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

I'd maybe have defaulted to these being optional, but we can always change again if that turns out to be the way to go. Looks good as-is, thanks!

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2022
@jetstack-bot
Copy link
Copy Markdown
Contributor

jetstack-bot commented Mar 17, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maelvls, SgtCoDFish

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot merged commit d04c464 into cert-manager:master Mar 17, 2022
@jetstack-bot
Copy link
Copy Markdown
Contributor

jetstack-bot commented Mar 17, 2022

@maelvls: Updated the job-config configmap in namespace default at cluster default using the following files:

  • key cert-manager-presubmits.yaml using file config/jobs/cert-manager/cert-manager/cert-manager-presubmits.yaml
Details

In response to this:

This PR is part of the effort 'moving away from Bazel' tracked in cert-manager/cert-manager#4712

This PR follows up the PR cert-manager/cert-manager#4914 that just got merged. As mentioned in the TODO,

# TODO: set to "always_run: true" and "optional: false" as soon as
# https://github.com/cert-manager/cert-manager/pull/4914 is merged.

This PR does exactly that.

Note that every open PR on cert-manager/cert-manager will have to get rebased in order to pass the two new tests pull-cert-manager-make-test and pull-cert-manager-make-e2e-v1-23. I will leave a message on each of the 33 open PR.

@munnerz made me aware of the fact that Prow runs the jobs on the merge commit, which means there is no need to rebase. But each PR (may?) have to get the required test manually triggered with the following:

/test pull-cert-manager-make-test
/test pull-cert-manager-make-e2e-v1-23

If that's the case, I will trigger these tests on each individual PR.

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.

@maelvls maelvls deleted the apk-add-grep-tar branch March 17, 2022 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SgtCoDFish SgtCoDFish SgtCoDFish approved these changes

Assignees

@SgtCoDFish SgtCoDFish

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cert-manager Indicates a PR related to cert-manager dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maelvls @jetstack-bot @SgtCoDFish