Rename ruleset as "CiviCRM" to be able to have default "Drupal" standard installed as well

[sluc23]

This ruleset is a variation of the Drupal default ruleset, but installed in phpcs under Drupal name. This blocks the possibility of installing default Drupal ruleset with the same name, to validate Drupal modules in the same machine.

Is there a way to have both ruleset installed in the same box where Drupal modules and CiviCRM core/extensions are developed? If not, wouldn't make sense the rename this ruleset as CiviCRM ?

[homotechsual]

+1 on this

[totten]

OK, so this and #15 are closely inter-related. I'm not exactly sure if it's better to post my comment here or there.. but...

It sounds like a question of how civicrm/coder should relate to upstream drupal/coder, eg

1. (Status quo) civicrm/coder is a soft fork of drupal/coder. It has some opt-outs -- and we periodically merge/rebase (like once every year or so).
2. civicrm/coder is a hard fork of drupal/coder. We can rename anything/everything, and we stop doing merges/rebases.
3. civicrm/coder is a separate package that depends on drupal/coder. It defines a separate "CiviCRM" ruleset which uses many of the sniffs from the Drupal ruleset.

Conceptually, it seems most appealing for civicrm/coder to be separate+dependent package. But I have no idea if that's easy or hard. (Bearing in mind that it's subjective... there's one POV for CI+civilint, and there's another POV for local IDEs.)

Relatedly, I know @eileenmcnaughton sometimes expresses frustration because the Civi+Drupal rulesets drift apart, and on the surface renaming would sorta harden that split. But I think maybe the real problem is the confusion that arises from mixing them (eg choosing the "Drupal" style in PHPStorm will give you warnings on a "CiviCRM" style codebase). So if we bite the bullet and make the split cleaner, perhaps that would address the same concern.

[eileenmcnaughton]

Hmm - I think my concern is that I want us to be periodically increasing our compliance - ie every now & then getting us in compliance with a new rule & locking it in. There are some places (class naming) where we are legitimately different but in general the drupal rules are ones we should adhere to & it's only because we haven't made existing code compliant that we don't

[homotechsual]

I think the suggestion 3 is the right way to approach this - build on top of drupal/coder - ignore the irrelevant rules, tone down the ones we need to tone down, override what we need to override.

Hopefully it would make maintenance easier in the long run and also gives us the ability to easily and cleanly do things like lint extension code automatically as part of extension review processes (speeding this up is a win as we can get more extensions approved for in-app!) it lets developers pull the standards into pretty much every editor/IDE these days (the amount of faff to get these standards in VSCode is a nightmare and knocks out the Drupal standards!) but more importantly hopefully it actually makes it easier to maintain/update as we're just worried about maintaining our code on top of the Drupal coder package as opposed to maintaining a fork.

I've started working locally on detangling the Drupal specific bits and instead referencing them from drupal/coder directly (which gets required as a dependency) - using the 8.x-2.x version at present.

Should I be rebasing this off of the 3.x.x version?

[totten]

From the change log, it sounds like the fundamental difference in 3.x.x is the target version of phpcs:

https://www.drupal.org/project/coder/releases/8.x-3.0

and of course a large number of subsequent patches to the sniffs/rules.

The main thing to look out for is the time required for balancing out the new sniffs and the code in core repos. Hopefully it can be trimmed without too much difficulty. This does seem like a good opportunity to modernize the phpcs version.

[homotechsual]

From the change log, it sounds like the fundamental difference in 3.x.x is the target version of phpcs:

https://www.drupal.org/project/coder/releases/8.x-3.0

and of course a large number of subsequent patches to the sniffs/rules.

The main thing to look out for is the time required for balancing out the new sniffs and the code in core repos. Hopefully it can be trimmed without too much difficulty. This does seem like a good opportunity to modernize the phpcs version.

Then that's the direction I'll go. Replicate where we are currently on 3.x.x in a new branch so we can test it against core code repos and see where we're at!

[michaelmcandrew]

hey @eileenmcnaughton @totten @homotechsual @sluc23 - interested in your thoughts on this: https://lab.civicrm.org/michaelmcandrew/civicrm-coding-standards/-/issues/1