feat: new github-membership script
#314
Conversation
src/github/membership/README.md
Outdated
| }; | ||
|
|
||
| (async () => { | ||
| const { success, failure } = await githubRelease(args); |
There was a problem hiding this comment.
| const { success, failure } = await githubRelease(args); | |
| const { success, failure } = await githubMembership(args); |
src/github/membership/index.ts
Outdated
| const org = (args.org || (await getRepository(args.path, args.spinner))?.owner)!; | ||
|
|
||
| if (args.users) { | ||
| const users = Array.isArray(args.users) ? args.users : [args.users]; |
There was a problem hiding this comment.
Isn't args.user always an array when defined?
There was a problem hiding this comment.
No, not when the CLI delivers a single user via garden github-membership -d jzempel
|
|
||
| interface IGitHubMembershipArgs { | ||
| org?: string; | ||
| users?: string[]; |
There was a problem hiding this comment.
| users?: string[]; | |
| users?: string | string[]; |
There was a problem hiding this comment.
Nah. For TS API usage, I think we want to force the developer to give us a string[]
| const options = command.opts(); | ||
| const results = await execute({ | ||
| org: options.org, | ||
| users: options.delete, |
There was a problem hiding this comment.
About this comment.
I'm a little confused. If users only accepts string[] | undefined, TS would throw - unless options.delete is of type any. Is that the case here?
There was a problem hiding this comment.
All commander.js options are of type any. The code we're swirling around here is simply a belt+suspenders mechanism to ensure users is being treated as the expected array and not turned into [j, z, e, m, p, e, l] for the limited corner case where the script would be used to delete one user. The intended & expected usage is bulk deletion. To say it another way:
- CLI could naturally be used to delete one or more users (and the script ensures type safety)
- API woud naturally be used to delete users in bulk
There was a problem hiding this comment.
Seems like commander.js always returns an array for variadic options. I’ll leave it up to you whether it’s worth removing the defensive line on L47.
There was a problem hiding this comment.
Hmm, seems like this was fixed in tj/commander.js#2410. I'll remove the over-aggressive defense.
| const options = command.opts(); | ||
| const results = await execute({ | ||
| org: options.org, | ||
| users: options.delete, |
There was a problem hiding this comment.
Seems like commander.js always returns an array for variadic options. I’ll leave it up to you whether it’s worth removing the defensive line on L47.
Description
This PR adds a new
garden github-membershipCLI, correspondinggithubMembershipAPI, and supporting documentation. See the new README for details.Detail
Used locally to scrub the
zendeskgardenmembership list – so we know it works as advertised 😉