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

Remove unused logging parameter. #63

Merged
merged 1 commit into from
Nov 22, 2016

Conversation

evocateur
Copy link
Contributor

Not that hasMatchingDependency() is ever called with logging enabled, but it was bothering me.

(Found it while trawling lerna pull requests)

@gigabo
Copy link
Contributor

gigabo commented Nov 21, 2016

Ugh, nice find. Those keep sneaking in with changes that are ported from Lerna.

Fortunately my loglevels patch was eventually applied to Lerna (under someone else's name), so we shouldn't see too many more of them.

@gigabo
Copy link
Contributor

gigabo commented Nov 22, 2016

I wonder where the call with showWarning=true used to be... 😕

@gigabo gigabo added the bug label Nov 22, 2016
@evocateur
Copy link
Contributor Author

The warning itself looks very similar to the root validation in the base Command class, but I wouldn't attach too much meaning behind the similarity.

@gigabo
Copy link
Contributor

gigabo commented Nov 22, 2016

Well... seeing as it's basically dead code now... maybe we can just remove it instead? 👹

@evocateur
Copy link
Contributor Author

Yeah, there's a lot of similar logging in BootstrapCommand, the only place this method is even used, it's probably safe to remove.

@evocateur
Copy link
Contributor Author

I decided to delete the unused block, it's just cleaner.

@evocateur evocateur changed the title Use correct method name for log warnings. Remove unused logging parameter. Nov 22, 2016
@evocateur evocateur merged commit d25a976 into asini:master Nov 22, 2016
@evocateur evocateur deleted the fix-logger-warning branch November 22, 2016 21:34
@gigabo
Copy link
Contributor

gigabo commented Nov 22, 2016

🤘

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

Successfully merging this pull request may close these issues.

None yet

2 participants