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

ownership, sig-buildsystem: add reviewers, add OWNERS files #11183

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

dhiller
Copy link
Contributor

@dhiller dhiller commented Feb 7, 2024

What this PR does

Before this PR:
The special files and folders for build system components, i.e. automation/ were lacking OWNERS, so that most PRs changing these were assigned to the global reviewers group.

After this PR:
Some more reviewers to the sig-buildsystem, also some OWNERS files are placed and the group of reviewers is extended. This makes reviews of PRs that change build system components to be assigned to the build system folks.

Fixes #

Why we need it and why it was done in this way

The following tradeoffs were made:
We surely have not hit everything in this first run, but follow up PRs can refine this matter.

The following alternatives were considered:

Links to places where the discussion took place: To be added

Special notes for your reviewer

  1. Not sure whether we want to have additional reviewers and approvers?
  2. prow, labels, github: add new labels for sigs used in OWNERS project-infra#3236 adds labels that will then get applied automatically

/cc @brianmcarey @enp0s3 @xpivarc

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

Release note

Extend OWNERS for sig-buildsystem

@kubevirt-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/M release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Feb 7, 2024
@dhiller dhiller changed the title Add sig-buildsystem owners and aliases Extend OWNERS for sig-buildsystem Feb 7, 2024
Copy link
Member

@xpivarc xpivarc left a comment

Choose a reason for hiding this comment

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

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xpivarc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 8, 2024
Copy link
Member

@brianmcarey brianmcarey left a comment

Choose a reason for hiding this comment

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

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2024
@dhiller dhiller changed the title Extend OWNERS for sig-buildsystem ownership, sig-buildsystem: add reviewers, add OWNERS files Feb 8, 2024
@dhiller
Copy link
Contributor Author

dhiller commented Feb 8, 2024

/cc @fabiand FYI

@kubevirt-bot
Copy link
Contributor

@dhiller: GitHub didn't allow me to request PR reviews from the following users: FYI.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @fabiand FYI

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.

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 8, 2024
@fabiand
Copy link
Member

fabiand commented Feb 12, 2024

owner parts lgtm

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2024
@dhiller
Copy link
Contributor Author

dhiller commented Feb 12, 2024

/verify-owners

@@ -0,0 +1,6 @@
reviewers:
Copy link
Member

Choose a reason for hiding this comment

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

cluster-up is a copy of the kubevirtci. I don't think we need a kubevirt owner for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though the process is largely automated, there are still situations where we need to look into things. My intent was

a) to be clear about who is responsible for the bumping process and
b) to make sure that sig-buildsystem is assigned to the kubevirtci bumps, so that problems arising are picked up directly by us.

Doesn't that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, makes sense. Thanks for explaining.

@dhiller dhiller marked this pull request as ready for review February 13, 2024 13:41
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2024
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@dhiller
Copy link
Contributor Author

dhiller commented Feb 14, 2024

🛑 please refrain from manual retesting.

⚠️ this PR is being held to stop automatic retesting. Quarantine PR #11244 is given priority.

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 14, 2024
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2024
@dhiller
Copy link
Contributor Author

dhiller commented Feb 14, 2024

/test pull-kubevirt-generate

@dhiller
Copy link
Contributor Author

dhiller commented Feb 15, 2024

/test pull-kubevirt-e2e-k8s-1.29-sig-storage

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 16, 2024
Adds some more reviewers to the sig-buildsystem, also places some OWNERS
files and extends the group of reviewers.

Furthermore modifies bump-kubevirtci to restore the OWNERS file after
actual update of kubevirtci.

Signed-off-by: Daniel Hiller <dhiller@redhat.com>
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 19, 2024
@dhiller
Copy link
Contributor Author

dhiller commented Feb 19, 2024

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2024
Copy link
Member

@brianmcarey brianmcarey left a comment

Choose a reason for hiding this comment

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

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 19, 2024
@kubevirt-commenter-bot
Copy link

Required labels detected, running phase 2 presubmits:
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-e2e-kind-1.27-vgpu
/test pull-kubevirt-e2e-kind-sriov
/test pull-kubevirt-e2e-k8s-1.29-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-network
/test pull-kubevirt-e2e-k8s-1.27-sig-storage
/test pull-kubevirt-e2e-k8s-1.27-sig-compute
/test pull-kubevirt-e2e-k8s-1.27-sig-operator
/test pull-kubevirt-e2e-k8s-1.28-sig-network
/test pull-kubevirt-e2e-k8s-1.28-sig-storage
/test pull-kubevirt-e2e-k8s-1.28-sig-compute
/test pull-kubevirt-e2e-k8s-1.28-sig-operator

@kubevirt-commenter-bot
Copy link

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@dhiller
Copy link
Contributor Author

dhiller commented Feb 20, 2024

Mild flake on pull-kubevirt-e2e-k8s-1.27-sig-compute

/override pull-kubevirt-e2e-k8s-1.27-sig-compute

Mild flake on pull-kubevirt-e2e-k8s-1.29-sig-operator

/override pull-kubevirt-e2e-k8s-1.29-sig-operator

@kubevirt-bot
Copy link
Contributor

@dhiller: Overrode contexts on behalf of dhiller: pull-kubevirt-e2e-k8s-1.27-sig-compute, pull-kubevirt-e2e-k8s-1.29-sig-operator

In response to this:

Mild flake on pull-kubevirt-e2e-k8s-1.27-sig-compute

/override pull-kubevirt-e2e-k8s-1.27-sig-compute

Mild flake on pull-kubevirt-e2e-k8s-1.29-sig-operator

/override pull-kubevirt-e2e-k8s-1.29-sig-operator

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.

@fabiand
Copy link
Member

fabiand commented Feb 20, 2024

/lgtm

What is missing here?

@dhiller
Copy link
Contributor Author

dhiller commented Feb 20, 2024

@fabiand not sure - seems tide is stuck somehow 🤷

@dhiller
Copy link
Contributor Author

dhiller commented Feb 20, 2024

Seems like prow is retesting #11109 (7e4fe04) by RamLavi
#11202 (828a7d2) by AlonaKaplan on a batch.

https://prow.ci.kubevirt.io/?repo=kubevirt%2Fkubevirt&type=batch

@kubevirt-bot
Copy link
Contributor

kubevirt-bot commented Feb 21, 2024

@dhiller: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubevirt-e2e-arm64 e66333a link false /test pull-kubevirt-e2e-arm64
pull-kubevirt-e2e-k8s-1.28-sig-storage 5b60624 link unknown /test pull-kubevirt-e2e-k8s-1.28-sig-storage

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. I understand the commands that are listed here.

@kubevirt-bot kubevirt-bot merged commit 4ae2d94 into kubevirt:main Feb 21, 2024
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants