[EasyCodingStandard] smaller sets over random imports + add BlankLineAfterStrictTypesFixer #443
Conversation
TomasVotruba
force-pushed
the
ecs-clean-sets
branch
from
75d9b1e
to
a08ab8d
Compare
| if ($this->skipper->shouldSkipCodeAndFile($fullyQualifiedCode, $this->path)) { | ||
| return false; | ||
| } | ||
|
|
||
| $this->addError($error, $stackPtr, $code, $data, $severity, true); |
There was a problem hiding this comment.
Maybe instead of your fix there should be
return $this->addError($error, $stackPtr, $code, $data, $severity, true) && $this->isFixer;What do you think?
There was a problem hiding this comment.
It felt bit weird to create duplicated code. This could be nicer.
Could you change it here? Just browser edit will do
TomasVotruba
force-pushed
the
ecs-clean-sets
branch
from
6971456
to
84defa4
Compare
|
@enumag I've failed to rebase on pull :/, so I've added your commit manually. |
TomasVotruba
changed the title
|
@TomasVotruba Can we backport this fix to 2.5? I tried to use 3.0-RC but you're still making BC breaks like removing sniffs so I'd like to stick to 2.5 until it's stable. With such changes 3.0 is alpha at best, I'd not call it RC. |
|
@enumag Sure. Feel free to create such branch and release patch tag with it. I recommend to wait until Symfony 4 is out. As for naming, I could try |
|
@TomasVotruba Ok I'll send a PR on Friday after SF4 is our. According to semver it should be like |
|
@TomasVotruba Also can I ask you for write access? I can't create the 2.5 branch without permissions... |
|
@enumag I've added you to collaborators with write access |
|
Thanks. I cherry-picked and pushed the commit to the 2.5 branch. v2.5.9...2.5 I'll try it on monday in my app to see if there is any other issue I need to fix in 2.5. Should I then tag 2.5.10 myself or leave that to you? I assume the tag will automatically appear in all the read-only repos? |
|
Great job! Feel free to tag yourself. To subsplit to single repositories, there is For speed reasons, it only includes master and last tag (not sure it by time or number), so maybe you'd have to modify it to include 2.5.10. |
|
Well I thought that this would do the trick but it did not. I'm not sure how to fix it. EDIT: Oh maybe it would work but didn't because the build failed? |
|
I did the subsplist with no effect. So I looked for the tag on monorepo I don't see the tag in releases: https://github.com/Symplify/Symplify/releases Where is it? |
|
I didn't tag it yet. The problems right now are that the 2.5 branch was not pushed to the split repos, locally one test is failing on that branch and there is no travis build for that branch. I can't tag without tests passing. |
|
I see. I've enabled "branch builds" check on Travis. 2.5 is now there |
|
Thanks! Ok travis is running and build is passing now. But the subsplit doesn't seem to work for that branch. How is the split command triggered? I thought it was on successful travis build but there is nothing about that in .travis.yml. |
|
You're welcome. The command is run manually, because it would require to add github API access keys and it's super slow. I can do it manually after you tag it. |
|
Oh. In that case I'm not quite sure if this change is correct. Maybe it should not have been changed and the script in master should be changed instead to |
|
If you need to publish only single tag, you'd have to name it explicitly: git subsplit publish --tags=2.5.10 packages/EasyCodingStandard:git@github.com:Symplify/EasyCodingStandard.gitAt the moment only tag of Symplify\Symplify. I will handle the rest. I see it's too complicated to explain here. I can explain it in person under 10 minutes 👍 |
|
Ok I reverted that commit and tagged the release. https://github.com/Symplify/Symplify/releases/tag/v2.5.10 |
|
@TomasVotruba Thank you! 👍 |
|
You're welcome |
Related Changes
FinalTestCase, useSlamCsFixer\FinalInternalClassFixerinsteadBlankLineAfterStrictTypesFixerTodo
make php7 set run only php7, dissable nullable and void types on sniff
update https://www.tomasvotruba.cz/blog/2017/08/21/5-useful-rules-from-symplify-coding-standard/