Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Update the used MediaWiki rule set to 0.12.0 #19

Merged
merged 1 commit into from Jan 26, 2018
Merged

Update the used MediaWiki rule set to 0.12.0 #19

merged 1 commit into from Jan 26, 2018

Conversation

thiemowmde
Copy link
Collaborator

I reviewed the new sniffs that come with this update and believe they are all fine. But feel free to disagree, and briefly explain your reasoning. Better disable one or two disputed sniffs than not updating, because all these updates also come with bugfixes.

  • LeadingZeroInFloat is not terribly important, but I think 0.1 is better readably than .1 and worth a sniff.
  • ClassMatchesFilename is obvious, I hope.
  • The new OneClassPerFile sniff does have a confusing name. It checks for classes, interfaces, and traits in one step and does not only replace the three individual sniffs, it does more that these: the individual sniffs are not able to detect cases with, for example, an interface and a class in one file.
  • OpeningKeywordBrace just got renamed.

Warning, this is a chain of patches! #16, #17, and #18 should be merged first!

@manicki
Copy link
Member

manicki commented Jan 26, 2018

The new OneClassPerFile sniff does have a confusing name. It checks for classes, interfaces, and traits in one step and does not only replace the three individual sniffs, it does more that these: the individual sniffs are not able to detect cases with, for example, an interface and a class in one file.

The name is not the most accurate, but I actually think it is OK like this.
From the commit message https://phabricator.wikimedia.org/rMCSN0da3ec7107c84909c1217dffa2300ccb6f580803

The separate three upstream sniffs would still have allowed a class and an interface to be in a file together, which still violates PSR-4.

@manicki manicki merged commit 85adaf3 into mwCs011 Jan 26, 2018
@manicki
Copy link
Member

manicki commented Jan 26, 2018

Also MW CS 0.12 dropped the Goat Sniffer integration, so I am not 100% glad of this change, but what better alternative do we have?

@manicki manicki mentioned this pull request Jan 26, 2018
@lucaswerkmeister
Copy link
Member

Well, removing more goats from more places would certainly be a better alternative, but if there are no more goats in the sniffers I guess this will have to do.

@thiemowmde
Copy link
Collaborator Author

All previous "one per file" sniffs have been replaced with Generic.Files.OneObjectStructurePerFile via #24.

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

Successfully merging this pull request may close these issues.

None yet

3 participants