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

nominate pacoxu as sig node reviewer #104186

Merged
merged 1 commit into from
Oct 5, 2021
Merged

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Aug 6, 2021

What type of PR is this?
/kind cleanup
/sig node

What this PR does / why we need it:
Requesting SIG Node reviewer status.

Special notes for your reviewer:
Completed reviewer requirements

To summarize what I have done that is related to sig-node:

NONE

/cc @Random-Liu @dchen1107 @derekwaynecarr @yujuhong @sjenning @mrunalp @klueska

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 6, 2021
@ehashman ehashman added this to Triage in SIG Node PR Triage Aug 6, 2021
@neolit123
Copy link
Member

+1 for @pacoxu

@endocrimes endocrimes moved this from Triage to Needs Approver in SIG Node PR Triage Aug 18, 2021
@pacoxu pacoxu changed the title Self nominate pacoxu as sig node reviewer nominate pacoxu as sig node reviewer Aug 26, 2021
@pacoxu

This comment has been minimized.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 2, 2021
@aojea
Copy link
Member

aojea commented Sep 5, 2021

+1

@derekwaynecarr
Copy link
Member

/assign

@derekwaynecarr
Copy link
Member

@pacoxu apologies for the lag on this issue. the SIG sub-project leads had tried to formulate a document for go forward criteria when evaluating reviewer and approver criteria.

/approve
/lgtm

To summarize the dimensions we looked for in reviewers which your nomination appears to capture from my perspective:

Committed

The 3 month of activities should be established looking at

  • PRs reviews history
  • Active participating in SIG Node or SIG Node CI weekly meetings or other transient meetings that arise when tackling specific problems in future (exceptions are allowed for cases when timezone or other personal limitations are not allowing for the meeting participation)

Technically sound

Proof of primary reviewership and significant contributions must be provided. Nominees must provide the list of PRs (at least 5 for primary reviewer and 20 substantial PRs authored or reviewed) as suggested in the membership document. Here are additional comments for this list of PRs:

  • Reviewed PRs must be merged.
  • Since the purpose is to demonstrate the nominee's technical depth, PRs like analyzer warnings fixes, mechanical “find/replace”-type PRs, minor improvements of kubelet logging and insignificant bug fixes are valued, but not counted towards the reviewer status nomination. Lack of reviews of those PRs may be a red flag for nomination approval.
    Making the most of reviews in a single area is a good indication that a subdirectory reviewer role may be preferred or top-level node reviewer. Finding the right balance helps balance the nominees future workload.
  • A primary reviewer should drive the review of the PR without significant input / guidance from the approver or other reviewers.

Has enough context

It is hard to assess codebase knowledge and it always will be a judgement call. SIG Node will rely on the listed PR to ensure the person reviewed PRs from different areas of SIG Node codebase and on the comments made during SIG Node meetings.

Additional ways to establish the knowledge of context are:

  • Contributions to k8s documentation
  • Blog posts - k8s-hosted and external
  • Talks at conferences and meetups
  • Contributions to other SIGs

Trustworthy

Reviewer nominations are accepted by SIG Node approvers. SIG Node approvers take nominations seriously and are invested in building a healthy community. Nominees should help approvers understand their future goals in the community so we can help continue to build trust and mutual relationships and nurture new opportunities if and when a contributor wants to become an approver!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2021
@pacoxu
Copy link
Member Author

pacoxu commented Oct 5, 2021

Thanks @derekwaynecarr for the detailed explaination.

/assign @thockin

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, pacoxu, thockin

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2021
@pacoxu
Copy link
Member Author

pacoxu commented Oct 5, 2021

/retest
/skip

@k8s-ci-robot k8s-ci-robot merged commit 6ee6251 into kubernetes:master Oct 5, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Oct 5, 2021
@pacoxu pacoxu deleted the patch-2 branch May 10, 2022 06:30
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Archived in project
SIG Node PR Triage
Needs Approver
Development

Successfully merging this pull request may close these issues.

None yet

6 participants