Skip to content

Revisit the OWNERS file for kubeadm#63551

Merged
k8s-github-robot merged 1 commit intokubernetes:masterfrom
luxas:revisit_kubeadm_owners
May 12, 2018
Merged

Revisit the OWNERS file for kubeadm#63551
k8s-github-robot merged 1 commit intokubernetes:masterfrom
luxas:revisit_kubeadm_owners

Conversation

@luxas
Copy link
Copy Markdown
Member

@luxas luxas commented May 8, 2018

What this PR does / why we need it:

The OWNERS file for kubeadm is getting a little bit stale. As discussed in today's SIG Cluster Lifecycle meeting, we're gonna update it with the currently active contributors.

Special notes for your reviewer:

Every person that is involved here, please ACK and LGTM the change.

@jbeda removed from approvers
@krousey removed from approvers/reviewers
@fabriziopandini graduated to an approver
@dmmcquay removed from reviewers
@jamiehannaford removed from reviewers
@Kargakis removed from reviewers
@liztio added to reviewers
@chuckha added to reviewers
@detiber added to reviewers
@stealthybox added to reviewers
@dixudx added to reviewers

Thank you everyone for your contributions 👏 (no one can't maintain something forever), and congratulations and welcome everyone with a new role, happy to have you here 👍!

Release note:

NONE

cc @kubernetes/sig-cluster-lifecycle-pr-reviews

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. retest-not-required-docs-only size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/kubeadm labels May 8, 2018
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 8, 2018
@0xmichalis
Copy link
Copy Markdown
Contributor

ack

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@luxas I think this needs to be a yaml comment (starts with # not //)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh true this is a yaml file. Will do

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@luxas I'd like to join in. Could you pls add me as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, in the SIG meeting I recalled you already were on this list and hence didn't mention adding you 😂, but yes, you should of course be here given the many reviews you've done and successfully participating in the pilot mentoring programme!

@neolit123
Copy link
Copy Markdown
Member

ACK 👍

@detiber
Copy link
Copy Markdown
Contributor

detiber commented May 8, 2018

ack

@timothysc
Copy link
Copy Markdown
Contributor

/lgtm
/hold

hold is to wait for timeout.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 8, 2018
@krousey
Copy link
Copy Markdown
Contributor

krousey commented May 8, 2018

ACK

@fabriziopandini
Copy link
Copy Markdown
Member

ack
/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, luxas, timothysc

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

The pull request process is described here

Details 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

@liztio
Copy link
Copy Markdown
Contributor

liztio commented May 9, 2018

nack

https://github.com/kubernetes/community/blob/master/community-membership.md this suggests reviewers should be members for at least 3 months, primary reviewer for 5+ PRs, and reviewed 20+ PRs. I don't meet any of those requirements, so I probably shouldn't be a reviewer @luxas

@neolit123
Copy link
Copy Markdown
Member

perhaps it would be possible to make an exception in a space where there is a need for more experienced reviewers.

@liztio
Copy link
Copy Markdown
Contributor

liztio commented May 10, 2018

sure, but I dunno that I count as "experienced" at this point :p

@jamiehannaford
Copy link
Copy Markdown
Contributor

ack

@luxas
Copy link
Copy Markdown
Member Author

luxas commented May 11, 2018

@liztio Being a "reviewer" in this case is more about being pinged for reviews on new PRs by the bot and officially having some kind of responsibility for reviewing PRs in that area. Being in this list doesn't give you either less or more power to actually merge things.The community membership guidelines are great, but given your dedication to kubeadm at the moment it doesn't make sense for me as a SIG lead to not include you here at this time just because you haven't yet come up to a specific number of reviews. I'm very confident you're gonna have done that number of reviews fairly soon, when pinged by the bot and from just burning down the open PRs stack with the rest of the kubeadm team.

Of course no one forces you to be on this list. It sounded like you're okay with it personally, but that you were just unsure if you met the requirements generally and now became hesitant if it's okay for others. Is that right? You have my support at least, and I'm sure you're gonna continue to do great work for us with your name here in this list 👍

@luxas luxas force-pushed the revisit_kubeadm_owners branch from 2cd4591 to 69cb1a5 Compare May 11, 2018 17:12
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2018
@stealthybox
Copy link
Copy Markdown
Member

ack ✅
thanks for updating the comment 👍

@stealthybox
Copy link
Copy Markdown
Member

/retest

@dmmcquay
Copy link
Copy Markdown
Contributor

ack 👍

@chuckha
Copy link
Copy Markdown
Contributor

chuckha commented May 11, 2018

ack

1 similar comment
@liztio
Copy link
Copy Markdown
Contributor

liztio commented May 11, 2018

ack

@jbeda
Copy link
Copy Markdown
Contributor

jbeda commented May 11, 2018

ack -- I effectively don't have time.

@luxas
Copy link
Copy Markdown
Member Author

luxas commented May 11, 2018

I effectively don't have time.

No worries!

@timothysc we have all ACK's needed, please re-apply the LGTM

@detiber
Copy link
Copy Markdown
Contributor

detiber commented May 11, 2018

/test pull-kubernetes-e2e-gce

@luxas
Copy link
Copy Markdown
Member Author

luxas commented May 12, 2018

Given that all needed ACK's are here, and that @timothysc lgtm'd this same PR earlier, I'm gonna let this merge.

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2018
@luxas luxas added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2018
@k8s-github-robot
Copy link
Copy Markdown

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit feeee50 into kubernetes:master May 12, 2018
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. area/kubeadm 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. release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.