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

feat: refactored config to fix precedence of config vs. args #388

Merged
merged 4 commits into from
Sep 13, 2016

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Sep 13, 2016

see #379

  • refactors config to simplify logic and fix order that args vs. config is applied.
  • should improve performance since yargs is only applied once now.

@mourner I will be landing your performance fixes to lib-instrument along with these other fixes, in the next nyc release.

reviewers: @addaleax, @JaKXz, @mourner

@mourner
Copy link
Contributor

mourner commented Sep 13, 2016

@bcoe the config refactor looks good, the only thing that doesn't fit usual JS conventions is the Config object, which is both a factory and a namespace, although typically capitalized names in JS are classes which are instantiated with new. For clarity, I'd turn the factory into another function (exports.makeConfig), and make the namespace lowercase (or maybe rename to config-util?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants