- Issue created by @quietone
- π³π΄Norway eiriksm Norway
And what would be the actual consequences for projects that has already changed the sort order to case sensitive. Would it have to be reverted? I guess I could figure it out somehow on my own, but currently on my phone and just wondering.
- π¬π§United Kingdom jonathan1055
[For those not on Slack, here is what I said on that thread]
I think it would be better to remove the entire Coder rule, not just the latest change which added case-sensitivity. Then when the standard is agreed, we can write the sniffs that implement exactly what we want. If we leave in place the sniff which does a plain sort, we'll be creating phpcs failures again for those projects which have just committed changes to satisfy case-sensitivity. We will create more annoyance than good, until the standard is fully settled, so best to remove the Coder rule entirely. - π©πͺGermany jurgenhaas Gottmadingen
@jonathan1055 I like that idea but also wonder if the sort mode couldn't be configurable on a per-project basis? But I don't know Coder and PHPCS well enough, so I might be completely off track here.
- π¬π§United Kingdom jonathan1055
Yes the case-sensitivity can be configured per project by adding the following to the project's phpcs.xml file
<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses"> <properties> <property name="caseSensitive" value="false"/> </properties> </rule>
The latest Coder commit added
property name="caseSensitive" value="true"
to the default Drupal coding standard config, so the lines above effectively revert that, but leave the original non-case-sensitive sort in place. This would be OK, but it means that
(a) the project maintainer has to do this change when they have already made a change to sort the statements case-sensitively,
(b) not all projects have their own phpcs.xml if they are OK with following all Drupal default standards. So we'd be forcing a project to add a phpcs.xml file just in the interim period, before the full standard is agreed, then the file may not be needed by the project when the new Coder sniffs are implemented. We'd then be forcing the maintainer to again make a change by deleting the phpcs.xml file, or removing that caseSensitve override.
(c) The is no guarantee that the final coding standard and Coder implementation of it will use the SlevomatCodingStandard sniff. There were certainly a few bugs in the phpcbf fixer for it it when we first worked on adding that standard. It might be that we write our own sniffs in Coder. Then that projects own phpcs.xml file would be referring to a sniff that is not used, making more confusion.All of these changes just introduce annoyance and unnecessary work, so I think the best thing to do at the moment is remove the rule entirely until the new standard is tested and written up and Coding Standards issue π Coding standards for "use" statements RTBC is fully agreed and completed.
- πΊπΈUnited States cmlara
I agree, given there is no official standard at the moment and the final standard is still very much undecided removing the sniff completely appears to be the best choice.
Updating IS/Title to reflect new proposal
- π¬π§United Kingdom jonathan1055
I have created a GitHub PR for this change
https://github.com/pfrenssen/coder/pull/240The issue summary says "Allow project to opt in to the order-by rule, with or without case sensitive sort" but this is not a thing that Coder can do. It just needs to be documented (somewhere) that the line
<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses">
can be added to the project's phpcs.xml file if they want to opt in. - π¬π§United Kingdom catch
Agreed, better to remove this then enable it once there's a proper standard.
- π¬π§United Kingdom jonathan1055
Hi @klausi,
The priority of this is set to 'major' and it has been RTBC for a week. Any chance you could commit it and make new Coder release. The longer the sniff stays in the more grief we are causing for developers, until the standard has been agreed.
Thanks. - πΊπΈUnited States cmlara
Anyone heard anything from @klausi? Should we ping on Slack?
- π¬π§United Kingdom jonathan1055
@klausi is in the #coder channel (I added him :-) but the other maintainers are not in that channel. I don't think it is fair to always ping one person if there are other maintainers too, we could add them there as that would be good, but I do not know which of these are active in Coder now:
- klausi
- pfrenssen
- arkener
- douggreen
- mikejw
- solotandem
- stella
- sun
- πΊπΈUnited States cmlara
I don't think it is fair to always ping one person if there are other maintainers too, w
Generally fair, I will note klausi has made 100% of the releases since 8.x-3.2 in April 2019.
GitLab Activity report indicates klausi is the only individual to have pushed commits into the repo in the past 2 years
https://git.drupalcode.org/project/coder/activityI believe we can call this (at least the D.O. side of the project) a 'single active maintainer' project.
-
jonathan1055 β
authored fd98546c on 8.3.x
fix(AlphabeticallySortedUses): Remove use statement order rule (#3483028...
-
jonathan1055 β
authored fd98546c on 8.3.x
- π³π±Netherlands arkener
Agreed, we should remove the sniff and revisit once π Coding standards for "use" statements RTBC is approved. I've merged the PR into 3.x, and I'll plan to create a new release this weekend, assuming no unexpected issues pop up. Thanks for the work on this!
- π¦πΉAustria klausi π¦πΉ Vienna
Sorry, was busy with other things. Thanks a lot for stepping up Arkener, love the Drupal community!
I will make a release now and explain in the release notes how people can add a snippet to their phpcs.xml file to keep sorted use statements. It is very useful in teams to better avoid git merge conflicts.
- π¦πΉAustria klausi π¦πΉ Vienna
Released: https://www.drupal.org/project/coder/releases/8.3.26 β
Thanks everyone!
Automatically closed - issue fixed for 2 weeks with no activity.