Skip to content

Conversation

spawnia
Copy link
Collaborator

@spawnia spawnia commented Dec 13, 2020

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.186% when pulling 7a171c6 on remove-unused-imports into 5225cb3 on master.

@mfn
Copy link
Contributor

mfn commented Dec 13, 2020

I was about to suggest why not enable no_unused_imports when I realized, it's not using php-cs-fixer; a feature sadly not supported by phpcs 😞

@simPod
Copy link
Collaborator

simPod commented Dec 13, 2020

@mfn phpcs supports this, I guess it's just not enabled here. #466 blocked by #470

@spawnia spawnia merged commit 5289a54 into master Dec 13, 2020
@mfn
Copy link
Contributor

mfn commented Dec 14, 2020

phpcs supports this

Does it? Granted, I'm not familiar with phpcs but squizlabs/PHP_CodeSniffer#801 leads me to assume otherwise.

@simPod
Copy link
Collaborator

simPod commented Dec 14, 2020

@mfn see https://github.com/slevomat/coding-standard#slevomatcodingstandardnamespacesunuseduses-

In PHPCS there IMO should not be any sniffs but the CS tool. Might lead to confusions like this.

@mfn
Copy link
Contributor

mfn commented Dec 14, 2020

confusions

I'm already confused by linking to coding-standard vs. the phpcs issue I linked but .. I totally trust your judgement, I feel already bad for derailing here so much. Apologies.

@simPod
Copy link
Collaborator

simPod commented Dec 14, 2020

There's phpcs tool for running sniffs and then there are coding standards that are basically rulesets composed of sniffs. Those rulesets are run via phpcs. And one such ruleset that is used here is maintained by Doctrine https://github.com/doctrine/coding-standard.

One confusing thing is that there are sniffs in phpcs repository https://github.com/squizlabs/PHP_CodeSniffer, I'd separate it into two 🤷🏾‍♀️ . Hope that clears it up a bit ;)

@spawnia spawnia deleted the remove-unused-imports branch March 27, 2021 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants