Skip to content
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

[Feat] Remove the need for Easy Coding Standard dependency #252

Merged
merged 5 commits into from Aug 25, 2019

Conversation

olivernybroe
Copy link
Sponsor Collaborator

@olivernybroe olivernybroe commented Aug 21, 2019

Q A
Bug fix? no
New feature? yes

This PR removes the need for the ECS dependency and uses phpcs directly.

This gives us a lot more control without having to change a lot of stuff in ECS as we are now in full control of the code that runs over all the files and the progress bar and so on...

This change has also been made for easily adding new static analysis tools as the current architecture was build around how ECS did stuff.

We now don't have the ErrorAndDiffCollector anymore as it was part of ECS. Instead when an error is happening the insight itself stores the information about the error.
So in the case of a sniff failing the error will be saved on the sniff itself. This also removes the "hack" we had in ErrorAndDiffCollector for getting the current sniff.

Of course another advantage of this change is that our dependencies are now less than before, without losing any features.

@olivernybroe olivernybroe requested a review from nunomaduro Aug 21, 2019
Copy link
Sponsor Collaborator Author

@olivernybroe olivernybroe left a comment

Added a few notes, to explain some of the stuff.

composer.json Show resolved Hide resolved
src/Application/Injectors/FileProcessors.php Show resolved Hide resolved
src/Domain/Contracts/FileProcessor.php Show resolved Hide resolved
src/Domain/File.php Show resolved Hide resolved
src/Domain/Insights/InsightFactory.php Show resolved Hide resolved
src/Domain/Insights/SniffDecorator.php Show resolved Hide resolved
src/Domain/Runner.php Show resolved Hide resolved
@nunomaduro
Copy link
Owner

@nunomaduro nunomaduro commented Aug 21, 2019

Looking forward to review this pull request. Looks promising!

Copy link
Owner

@nunomaduro nunomaduro left a comment

It's all perfect, this is a huge improvement. Thank you for this.

src/Domain/Contracts/FileProcessor.php Show resolved Hide resolved
src/Domain/Contracts/Repositories/FilesRepository.php Outdated Show resolved Hide resolved
src/Domain/Runner.php Show resolved Hide resolved
@nunomaduro nunomaduro added the enhancement label Aug 22, 2019
@nunomaduro nunomaduro merged commit 92cf29a into nunomaduro:master Aug 25, 2019
1 check passed
@nunomaduro
Copy link
Owner

@nunomaduro nunomaduro commented Aug 25, 2019

🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants