Skip to content

[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

Merged
merged 1 commit into from
Jun 29, 2016

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jun 23, 2016

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.

@chalasr chalasr force-pushed the patch_yaml_lintcommand branch 4 times, most recently from 130fa67 to 5f7ad81 Compare June 23, 2016 22:05
{
$countFiles = count($filesInfo);
$erroredFiles = 0;

foreach ($filesInfo as $info) {
if ($info['valid'] && $output->isVerbose()) {
if ($info['valid'] && $this->outputCorrectFile) {
Copy link
Member Author

@chalasr chalasr Jun 23, 2016

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?

Copy link
Member

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
@chalasr chalasr force-pushed the patch_yaml_lintcommand branch from 5f7ad81 to dd84b7f Compare June 24, 2016 09:18
{
$countFiles = count($filesInfo);
$erroredFiles = 0;

foreach ($filesInfo as $info) {
if ($info['valid'] && $output->isVerbose()) {
if ($info['valid'] && $this->displayCorrectFiles) {
Copy link
Member

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()

Copy link
Member Author

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?

Copy link
Member

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
Copy link
Member

fabpot commented Jun 29, 2016

Thank you @chalasr.

@fabpot fabpot merged commit dd84b7f into symfony:master Jun 29, 2016
fabpot added a commit that referenced this pull request Jun 29, 2016
… 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
@chalasr chalasr deleted the patch_yaml_lintcommand branch June 29, 2016 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants