Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Yaml] Avoid using both Input/Output and SymfonyStyle in LintCommand#19160
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
130fa67 to5f7ad81Compare| foreach ($filesInfoas$info) { | ||
| if ($info['valid'] &&$output->isVerbose()) { | ||
| if ($info['valid'] &&$this->outputCorrectFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should I check output verbosity too for per-file errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No, errors should always be displayed.
Remove input/output from method arguments, use SymfonyStyle insteadAvoid 'error in filename' if no filename as argumentAdd missing array typehint to files argument of display*() methodsStore format option as member rather than pass it in method callsCS fixes
5f7ad81 todd84b7fCompare| foreach ($filesInfoas$info) { | ||
| if ($info['valid'] &&$output->isVerbose()) { | ||
| if ($info['valid'] &&$this->displayCorrectFiles) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would remove thedisplayCorrectFiles property entirely and replace it with$io->isVerbose()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@javiereguiluz I was glad to use it, but it make tests failing as the verbosity methods are available since3.1 only, dev requirements are to~2.8|~3.0. Should I up the version bound to~3.1 for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm sorry I didn't think about that. Let's keep the property then, even its use looks so limited.
fabpot commentedJun 29, 2016
Thank you@chalasr. |
… LintCommand (chalasr)This PR was merged into the 3.2-dev branch.Discussion----------[Yaml] Avoid using both Input/Output and SymfonyStyle in LintCommand| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | ~| License | MIT| Doc PR | ~Fixed some inconsistencies/mistakes from the original YamLintCommand.Commits-------dd84b7f [Yaml] Avoid using both Input/Output and SymfonyStyle in LintCommand
Fixed some inconsistencies/mistakes from the original YamLintCommand.