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 jpbetz to prod-readiness-approvers #4343

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Nov 22, 2023

Shadow contribution summary:

  • 1.27 shadow reviews: 6
  • 1.28 shadow reviews: 9
  • 1.29 - did not shadow due to time constraints with new SIG API Machinery TL role

Planned future involvement:

  • 1.30+ contribute to PRR, able to review roughly 12 KEPs per release. I am happy to PRR more than 12 so long as they are SIG API Machinery KEPs, since I'll be reviewing those anyway..

Shadow reviewer promotion criteria:

Transitions from new to alpha

Transitions from alpha to beta

Transitions from beta to GA

Three enhancements that require coordination between multiple components.

Three enhancements that require version skew consideration (both HA and component skew): does behavior fail safely and eventually reconcile.

Three enhancements that are outside your primary domain.

Examples where the feature requires considering the case of administering thousands of clusters. This comes up frequently for host-based features in storage, node, or networking.

Examples where the feature requires considering the case of very large clusters. This is commonly covered by metrics.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 22, 2023
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 22, 2023
@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 22, 2023

@kikisdeliveryservice
Copy link
Member

/assign @johnbelamaric @deads2k @wojtek-t

@jeremyrickard
Copy link
Contributor

@jpbetz in the prr meeting on Wednesday, there was discussion about linking to review comments in PRs to add ourselves to approvers so it would simply the review. Could you update the issue description with links to the review comments for the issues you linked?

@jpbetz
Copy link
Contributor Author

jpbetz commented Dec 1, 2023

Could you update the issue description with links to the review comments for the issues you linked?

I've added links to the PRs reviewed and to the review comments.

@johnbelamaric
Copy link
Member

/approve

Welcome :)

@deads2k @wojtek-t I look to you for final approve and LGTM!

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 14, 2023
@wojtek-t
Copy link
Member

Now being pedantic here:

  • I think that from the formal POV all the explicit criteria are satisfied
  • I didn't see any case [though please let me know if I missed that] of going back-and-forth with the KEP author [so kind-of addressing this: https://github.com/Clarify PRR approvers criteria community#7621 ]
  • some things that I found are:

I don't think the minor stuff about should be blocking, but I'm wondering about the second bullet point above [though if we consciously make a "non-blocking" decision, I would be fine with that too, but primarily because I know Joe and I've seen him pushing back in other situations].

@johnbelamaric
Copy link
Member

johnbelamaric commented Dec 15, 2023

I thought this showed a bit of that:

#3780 (review)

@wojtek-t
Copy link
Member

I thought this showed a bit of that:

I think it's good example of questioning status-quo, but not necessary pushing for sth.

[As I said, I'm not saying we should necessarily block on that, just trying to figure out where we're putting the bar, so having @deads2k opinion here would be helpful]

@jpbetz
Copy link
Contributor Author

jpbetz commented Jan 9, 2024

Friendly ping on this.

@deads2k
Copy link
Contributor

deads2k commented Jan 11, 2024

The diversity of review requirements is well met. Regarding the ability to go back and forth with authors to explain and (as gently as possible) require the project standards to be met. While it's ideal to see that in an PRR review that required it (and a significant negative if it was necessary and lacking), I'm comfortable using our five year history and knowledge of this candidate to fulfill the requirement. I can't guarantee that existing approvers will have that knowledge of all future candidates and if we don't have it we won't be able to use it for future candidates.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 11, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, johnbelamaric, jpbetz

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 merged commit 567de61 into kubernetes:master Jan 11, 2024
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Jan 11, 2024
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants