Skip to content
This repository has been archived by the owner on Nov 21, 2022. It is now read-only.

Extensively rework dynector to use yargs commands. #74

Merged
merged 4 commits into from
Sep 15, 2016
Merged

Conversation

ceejbot
Copy link
Contributor

@ceejbot ceejbot commented Sep 14, 2016

The new usage is given in the README and as help. It uses yargs commands intead of named options. Specifically:

dyn arecord <fqdn> <ip> make the given fqdn resolve to the given IP
dyn cname <fqdn> <cname> make the given fqdn resolve to the given cname
dyn delete <fqdn> remove the named node entirely
dyn list <zone> list all records for the given zone
dyn resolve <zone> resolve all records for the given zone; list the IPs

Reworked the tests.

Added list, which lists all records for the given zone, with types.
Added resolve, which resolves all records for the given zones and emits a sorted list of all IPs in use.

Added support for update-notifier.

This will of course require a major-number version bump when landed.

The new usage is given in the README and as help. It uses
yarg commands intead of named options. Specifically:

dyn arecord <fqdn> <ip>   make the given fqdn resolve to the given IP
dyn cname <fqdn> <cname>  make the given fqdn resolve to the given cname
dyn delete <fqdn>         remove the named node entirely
dyn list <zone>           list all records for the given zone
dyn resolve <zone>        resolve all records for the given zone; list the IPs

Reworked the tests.

Added `list`, which lists all records for the given zone, with types.
Added `resolve`, which resolves all records for the given zones and
emits a sorted list of all IPs in use.

Added support for update-notifier.

This will of course require a major-number version bump when landed.
@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+27.5%) to 73.394% when pulling ae49528 on ceej/commands into 32451db on master.

.option('zone',
{
alias: 'z',
description: 'specify a zone',
Copy link

Choose a reason for hiding this comment

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

These options should be marked global: true to apply to all commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! Exactly the kind of feedback I wanted from you!

describe: 'do not log informationally',
type: 'boolean'
})
.example('dynector arecord example.com 10.0.0.1')
Copy link

Choose a reason for hiding this comment

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

Examples would look slightly better in the terminal if each one had a brief description, as second argument to each .example() call.

@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+27.5%) to 73.394% when pulling cda54c8 on ceej/commands into 32451db on master.

.example('dynector delete oops.example.com', 'remove the entry for oops entirely')
.example('dynector list example.com', 'list all records under example.com')
.example('dynector resolve example.com', 'resolve IPs for all example.com records')
.version()
Copy link

Choose a reason for hiding this comment

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

With yargs 5, you can now do .recommendCommands() to provide a suggestion if someone mistypes a command. 👍

@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+27.5%) to 73.394% when pulling f7b806a on ceej/commands into 32451db on master.

@coveralls
Copy link

coveralls commented Sep 15, 2016

Coverage Status

Coverage increased (+27.5%) to 73.394% when pulling 13f9874 on ceej/commands into 32451db on master.

Copy link
Contributor

@mmalecki mmalecki left a comment

Choose a reason for hiding this comment

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

Looks good to me! This shouldn't change much in scripting, makes the UX better as well.


yargs.recommendCommands();

yargs.argv;
Copy link
Contributor

Choose a reason for hiding this comment

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

So accessing argv here effectively starts the application?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. This is how yargs commands work.

@bcoe
Copy link

bcoe commented Sep 15, 2016

😮 this is really cool! neat to see all the fancy new command functionality being put to good use.

@ceejbot ceejbot merged commit dc2b230 into master Sep 15, 2016
@ceejbot ceejbot deleted the ceej/commands branch September 15, 2016 17:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants