Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for blunderbuss to wait for tide status #349

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dobesv
Copy link
Contributor

@dobesv dobesv commented Dec 18, 2024

As a code reviewer, I don't want to review code which has not passed the test suites yet.

If I am asked to review something and then the tests fail after my review, it feels like I wasted my time as the person submitting it has to address the feedback from the tools anyway. However, if I do not review it immediately then I have to manually check back later to see whether the tests have passed - there is no way currently to set up a notification for tests passing under the current system.

This change allows configuring blunderbuss to request reviews when a matching commit status is reported instead of immediately when the pull request is opened or marked as ready for review. By setting the condition to one which matches the status that tide posts when all tests have passed but it is waiting for a label or for GitHub reviews to be posted, I can defer the automated review requests until the tests have passed.

This way someone can open a pull request, and when they eventually get all the tests passing, I can be prompted to review the pull request. I won't waste time reviewing a pull request that has failing tests or lint errors, and I also will not have to manually "check back" on all the pull requests to see which ones have the tests passing.

For example in this project, you could in theory set up blunderbuss to wait with requesting reviews until tide shows this status:

image

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 18, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @dobesv. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 18, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 18, 2024
Copy link

netlify bot commented Dec 18, 2024

Deploy Preview for k8s-prow ready!

Name Link
🔨 Latest commit 71eb2c7
🔍 Latest deploy log https://app.netlify.com/sites/k8s-prow/deploys/67786206cb661200080ea9a9
😎 Deploy Preview https://deploy-preview-349--k8s-prow.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@matthyx
Copy link
Contributor

matthyx commented Dec 18, 2024

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 18, 2024
@dobesv
Copy link
Contributor Author

dobesv commented Dec 19, 2024

/retest

@dobesv
Copy link
Contributor Author

dobesv commented Dec 19, 2024

I probably would need some guidance how to resolve that failure in pull-prow-integration, I'm having trouble relating that failure to my change here.

@hashim21223445 hashim21223445 mentioned this pull request Dec 28, 2024
VannTen and others added 2 commits January 2, 2025 16:10
Currently, the command-help page renders multiline command examples (the
only example I've seen is for /release-note-edit) on one line, which is
confusing for users.

Change the style to respect newline in the source.

Link: https://developer.mozilla.org/en-US/docs/Web/CSS/white-space
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dobesv
Once this PR has been reviewed and has the lgtm label, please assign cjwagner, michelle192837 for approval. For more information see the Kubernetes Code Review Process.

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants