-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
130fa67
to
5f7ad81
Compare
{ | ||
$countFiles = count($filesInfo); | ||
$erroredFiles = 0; | ||
|
||
foreach ($filesInfo as $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 instead Avoid 'error in filename' if no filename as argument Add missing array typehint to files argument of display*() methods Store format option as member rather than pass it in method calls CS fixes
5f7ad81
to
dd84b7f
Compare
{ | ||
$countFiles = count($filesInfo); | ||
$erroredFiles = 0; | ||
|
||
foreach ($filesInfo as $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 the displayCorrectFiles
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 since 3.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.
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.