-
Notifications
You must be signed in to change notification settings - Fork 356
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
override new phpcs rule #465
Conversation
915fdd1
to
7eb3841
Compare
.travis.yml
Outdated
@@ -10,6 +10,7 @@ matrix: | |||
fast_finish: true | |||
include: | |||
- php: 5.3 | |||
dist: precise |
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.
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.
This is already fixed in 6.0.0-dev.
Won't forcing this to post, rather than simply disabling it, potentially introduce logic errors in the future? Changing the order of an increment / decrement operation in any situation that assigns the result somewhere is asking for trouble, as they are not equivalent. Can we turn this rule off completely so that it leaves increment / decrement alone? I don't trust php-cs-fixer to recognize assignment situations, and it's introduced problems with this kind of assumption in the past. |
Note the comments above regarding assignment also include using the result as input to another logic operation, regardless of whether a variable is explicitly set. |
7eb3841
to
e3c42d5
Compare
that’s true, it disabled it now, unfortunately it seems there is issue when running |
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.
Looks good 👍
.php_cs.dist
Outdated
@@ -24,6 +24,7 @@ $config | |||
'pre_increment' => false, | |||
'trailing_comma_in_multiline_array' => false, | |||
'simplified_null_return' => false, | |||
'increment_style' => false, |
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.
Thanks :-)
Hooray, more dependency issues! Will take a look, hang on. |
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.
Dependency issues are historic, and are a result of this PR being opened against the wrong branch.
Please change the base branch for this PR to 6.0.0-dev.
e3c42d5
to
cccf266
Compare
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.
Perfect - thanks 👍
my bad, I even saw the comment #390 (comment) a few days ago and didn’t remember…, so that’s much better now I don’t have much time to help more so I just wanted to do this to get some work of your plate (even if it’s not much), thx for the all work ;) |
All good - I appreciate your taking care of this :-). |
No description provided.