-
Notifications
You must be signed in to change notification settings - Fork 40k
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 a notice for node e2e config files #54328
Conversation
@krzyzacy: Adding do-not-merge/release-note-label-needed because the release note process has not been followed. One of the following labels is required "release-note", "release-note-action-required", or "release-note-none". 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. |
/release-note-none |
/kind e2e |
test/e2e_node/jenkins/OWNERS
Outdated
@@ -0,0 +1,3 @@ | |||
reviewers: | |||
- sig-node-reviewers | |||
- krzyzacy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it help to add a approver section with a smaller set of approvers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only try to subscribe to some notifications when people are touching those config/property files so I added myself as a reviewer for now. If you think approves are better I can make the change :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a reviewer, you'd get only a small number of PRs.
I think it makes sense to add an approver section, and put a couple of people who are aware of the situation there.
(okay, updated and github recovered from 🦄) |
/lgtm @dashpole @Random-Liu @yguo0905 please ack for being added as an approver. |
ack |
/retest |
/approve no-issue |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krzyzacy, yujuhong Associated issue requirement bypassed by: yujuhong The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here. |
ref #53542 and patched up with kubernetes/test-infra#5107
So while migrating the jobs to prow, I haven't kill the
*.properties
files yet because some lingering jobs, and possibly local tests are still using them. We have a copy of image-config.yaml in test-infra, and all *.properties file is merged into job configs.Add a notice to remind people also update the job configs in test-infra. Also add myself as a reviewer here so I can subscribe some notice. I'll remove them once I cleaned up all legacy files here.
/assign @yguo0905 @dashpole @yujuhong