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

fix: correctly load external config file #294

Merged
merged 1 commit into from
May 16, 2019

Conversation

dlouzan
Copy link
Contributor

@dlouzan dlouzan commented Feb 21, 2019

  • external configuration files are properly handled
  • transform verbose parameter in config file to a 'loglevel' integer
    that can be increased. uniformity in handling of verbose param
  • add test cases

@dlouzan
Copy link
Contributor Author

dlouzan commented Feb 21, 2019

The current master is not able to load a configuration file passed with the -c option; I don't know if this is an upstream issue in commander, but the definition must use the .parent object, since it's defined on top of the command hierarchy.

Additionally, I made the handling of the verbose option more uniform, since each command was doing its own magic. One of the most recent changes made the verbosity a log level, so it is treated as an integer in code (1 is normal output, 2+ print debug information).

Lastly, the promise chain of each command will now show an error message if some bad exception happens. The current code will just swallow those and exit with an error status.

@pderaaij
Copy link
Contributor

@djtarazona this one solves some problems with the current release. @dlouzan could you perhaps update the PR and resolve the conflicts?

- external configuration files are properly handled
- transform verbose parameter in config file to a 'loglevel' integer
  that can be increased. uniformity in handling of verbose param
- add test cases
@dlouzan dlouzan force-pushed the fix/external-config-file-load branch from 9f7cf5e to 6ab5082 Compare May 15, 2019 14:53
@dlouzan
Copy link
Contributor Author

dlouzan commented May 15, 2019

@pderaaij I've just updated the PR. Only some issues with package lock.

@djtarazona djtarazona merged commit bd1e10a into wework:master May 16, 2019
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.

3 participants