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

Normalize httpsOptions from parseCli #644

Merged
merged 3 commits into from
Aug 31, 2016
Merged

Normalize httpsOptions from parseCli #644

merged 3 commits into from
Aug 31, 2016

Conversation

jbenesch
Copy link
Contributor

@jbenesch jbenesch commented Aug 29, 2016

See: #569

Most checks in the codebase, check for httpsOptions and not the https flag. This PR ensures httpsOptions is always defined and set from parseCli.

  • I also added a babel plugin called add-module-exports to make things easier when importing default exports.
  • I added a package called ava for tests (following the generator package lead).
  • I moved the pem package from the start command over to parseCli, so we can get a standard httpsOptions object (or boolean) in cli.start.
  • Removed the camelize function from parseCli, as the yargs package already does that for us.
  • Wrote 24 unit tests. It's still not complete and will work on adding more tests around all cli flags.
  • Linter passes.

const defaultOptions = require("./defaultOptions").default;
const run = require("./run");
const parseCliArgs = require("./parseCliArgs");
const defaultOptions = require("./defaultOptions");
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, that's nice! Didn't know about babel-plugin-add-module-exports.

@gigabo gigabo added the bug An issue with the system label Aug 31, 2016
@gigabo gigabo merged commit 3604953 into redfin:master Aug 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants