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

[WIP] Add remove-members helper script #2048

Closed

Conversation

mrbobbytables
Copy link
Member

This is the script used to generate #2047
It needs some final polish like usage information, but figured I'd share for initial review / vetting alongside #2047

/hold

@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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/github-management Issues or PRs related to GitHub Management subproject labels Jul 18, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrbobbytables

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 Jul 18, 2020
Copy link
Member

@nikhita nikhita left a comment

Choose a reason for hiding this comment

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

Left some suggestions, thanks for doing this ❤️


readonly REPO_ROOT="$(git rev-parse --show-toplevel)"
readonly CONFIG_PATH="$REPO_ROOT/config"
readonly DRYRUN="${DRYRUN:-true}"
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this a flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this was me being kind of lazy to get it done lol

set -o pipefail

readonly REPO_ROOT="$(git rev-parse --show-toplevel)"
readonly CONFIG_PATH="$REPO_ROOT/config"
Copy link
Member

Choose a reason for hiding this comment

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

Can/should we make this configurable? In cases where someone is active in kubernetes-sigs but not active in kubernetes, or if we just want to remove members from kubernetes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can do that 👍

@nikhita
Copy link
Member

nikhita commented Jul 20, 2020

/assign @cblecker

@mrbobbytables
Copy link
Member Author

There is one issue I just thought of, it currently won't remove members if they have a comment after their name like
- foo # herp derp
Lucked out with the original batch

@fejta
Copy link
Contributor

fejta commented Jul 20, 2020

Thoughts on putting this somewhere aside from hack? The unfortunately named hack folder generally seems to be used for lint-type updates and/or verifications.

Isn't this an operational thing? AKA I want to remove Erick from the org and this is the tool to do so, which just happens to be in bash right now.

@justaugustus
Copy link
Member

justaugustus commented Jul 20, 2020

Adding on to @fejta's point, I think this is important enough and of sufficient complexity that it should be written in Go.

@mrbobbytables
Copy link
Member Author

I'm okay with that -- I'm a little low on time at the moment, but I'll see what I can do to get something a bit better pushed up. Either way will get something before the next round of clean up. 👍

/close

@k8s-ci-robot
Copy link
Contributor

@mrbobbytables: Closed this PR.

In response to this:

I'm okay with that -- I'm a little low on time at the moment, but I'll see what I can do to get something a bit better pushed up. Either way will get something before the next round of clean up. 👍

/close

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.

@nikhita
Copy link
Member

nikhita commented Jul 21, 2020

Created #2052 so that we don't forget this. I'm also happy to take a look at it later :)

@mrbobbytables mrbobbytables deleted the remove-member-script branch September 17, 2020 14:06
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/github-management Issues or PRs related to GitHub Management subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants