Skip to content

Changed phpcsfixer2 default options#471

Merged
Landerstraeten merged 1 commit intophpro:masterfrom
vyshkant:phpcsfixer2_defaults
Feb 26, 2018
Merged

Changed phpcsfixer2 default options#471
Landerstraeten merged 1 commit intophpro:masterfrom
vyshkant:phpcsfixer2_defaults

Conversation

@vyshkant
Copy link
Contributor

Q A
Branch master for features and deprecations
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Documented? yes
Fixed tickets #469

Existed non-null default values of "allow_risky" and "using_cache" options were overriding the values from .php_cs config.

The current behavior is that if these options are not specified, the values will be taken from the .php_cs config.

Copy link
Contributor

@veewee veewee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

$arguments->add('--dry-run');
$arguments->addOptionalArgument('--allow-risky=%s', $config['allow_risky'] ? 'yes' : 'no');

if (null !== $config['allow_risky']) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to avoid these if statements inside the tasks. Can you move this nullable argument to a new $arguments->add* method?

Copy link
Contributor Author

@vyshkant vyshkant Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to avoid these if statements inside the tasks. Can you move this nullable argument to a new $arguments->add* method?

@veewee Done!

$this->getValues()->shouldBe(['--argument=file1.txt,file2.txt']);
}

function it_should_be_able_to_add_boolean_nullable_argument()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make three separate tests. Now you need to take the previous result into account.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Landerstraeten sure! Don't go anywhere, I'll fix it right now :)

Existed non-null default values of "allow_risky" and "using_cache" options were overriding the values from .php_cs config.

Closes #469
@Landerstraeten Landerstraeten merged commit 1de3f62 into phpro:master Feb 26, 2018
@Landerstraeten
Copy link
Contributor

@vyshkant thank you!

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.

3 participants