Closed mjgiarlo closed 5 years ago
I agree with option 1. Feel free to merge it yourself or to deal with coverage using option 1 and then merge it.
I will do the latter. Pls hold off on merging.
Chatted with @ndushay about the most recent commit in this branch and she's OK with my merging this given her prior approval. Merging now.
Includes
commander
bin/acl
executable to packagepackage.json
:main
entryeslint
on all .js files (future-proofing in case we add dirs)--runInBand
arg from jest invocations (unnecessary AFAICT?)jest-integration
npm script, setting up near-term integration workQuestion
Due to the way
commander
is tested—using another process per a recipe found on Google—none of the coverage added in this branch is picked up by static analysis. :unamused: But it's there! We can handle this in one of three ways:src/cli/commander.js
for code coverage. Possibly defensible, since we shouldn't need to test thecommander
library insinopia_acl
.commander
usage.I lean towards 1, for expediency's sake.
Notes
Note 1: I chose to use
commander
for the CLI since it's both featureful and maintained unlike some of the other libraries mentioned in #4.Note 2: I considered using
npm run
instead ofbin/...
for this, but so far I'm preferring thebin/...
approach to distinguish it from the developer experience of running stuff withnpm run
.Design
Here's how the CLI looks from the user perspective.
Print help if no commands or options provided:
Print help when requested (also works with
-h
):Print version of acl tool (taken from package.json):
Show all groups:
Throw an error when required argument is missing:
List all users in a group:
Add a user to a group:
Remove a user from a group:
fixes #4