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

Respect verbosity levels #265

Merged
merged 2 commits into from
Feb 18, 2019
Merged

Respect verbosity levels #265

merged 2 commits into from
Feb 18, 2019

Conversation

pderaaij
Copy link
Contributor

Ensures that verbosity levels are respected and passed on to dependent modules.

@pderaaij
Copy link
Contributor Author

@smythey21 Can you have a look at this? Can't add you as a reviewer

@pderaaij
Copy link
Contributor Author

@smythey21 unfortunately I can't merge pull requests as I don't have write access. Can you press the button?

@pderaaij
Copy link
Contributor Author

@mdaloia Thanks, wasn't aware of the default value argument. Changed it.

@mdaloia
Copy link
Contributor

mdaloia commented Jan 31, 2019

@pderaaij You're welcome!

I have one doubt. Maybe I'm not seeing something but, aren't you now ignoring the verbose config and honoring just the quiet config?

Based on your original modification:

const verbose = config.get('quiet') ? 0 : (config.get('verbose') ? config.get('verbose') : 1);

what I had in mind about was to change it to something like this:

const verbose = config.get('quiet') ? 0 : config.get('verbose', 1);

@pderaaij
Copy link
Contributor Author

I should not code before I have my first coffee. Fixed, thanks

@pderaaij
Copy link
Contributor Author

pderaaij commented Feb 5, 2019

@djtarazona Can you have a look at this PR and merge?

@djtarazona
Copy link
Contributor

@pderaaij I will review this weekend. Sorry for the delay!

@pderaaij
Copy link
Contributor Author

pderaaij commented Feb 8, 2019

@djtarazona No problem, let me know how I can help work away the open Pull Requests...

@pderaaij
Copy link
Contributor Author

@djtarazona or @smythey21 any update?

@djtarazona
Copy link
Contributor

@pderaaij Sorry, I will merge as soon as I get some permission issues for this repo sorted out.

@smythey21 smythey21 merged commit 7274d72 into wework:master Feb 18, 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.

4 participants