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.

Sign up for GitHub

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

Switch Acquia PHP to PSR12 #68

Closed
wants to merge 14 commits into from
Closed

Switch Acquia PHP to PSR12 #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
430d8c2
Switch Acquia PHP to PSR12
danepowell Mar 28, 2024
e66e673
Add edge standards
danepowell Mar 28, 2024
49a1d94
typo
danepowell Mar 28, 2024
92d5cab
Add slevomat standards
danepowell Mar 28, 2024
d484975
fix test
danepowell Mar 28, 2024
94d545e
fix tests
danepowell Mar 28, 2024
7d9894a
fix tests
danepowell Mar 28, 2024
a78ddaa
fix tsets
danepowell Mar 28, 2024
51835ae
Fix class declaration
danepowell Mar 29, 2024
3dad619
fix function declarations
danepowell Mar 30, 2024
b2d409a
functioncallsignature
danepowell Apr 1, 2024
9105771
control structure spacing
danepowell Apr 1, 2024
4474fe7
controlsignature
danepowell Apr 1, 2024
3b802da
cleanup
danepowell Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ if [[ "$FAILURES" ]]; then
fi

# Place a good test file.
printf "<?php\n\n/**\n * @file\n * Good test file.\n */\n" > good.php
printf "<?php\n\n/**\n * @file\n * Good test file.\n */\n\ndeclare(strict_types=1);\n" > good.php

# Test that the SUT's standards can be run.
EXPECTED=(
Expand Down
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ Acquia Coding Standards for PHP includes a selection of sniffs from the followin

Rules are split into rulesets according to the project language and framework:

* [AcquiaPHP](src/Standards/AcquiaPHP/ruleset.xml) contains sniffs applicable to all PHP projects.
* [AcquiaDrupalStrict](src/Standards/AcquiaDrupalStrict/ruleset.xml) incorporates AcquiaPHP and adds all Drupal coding standards and best practices sniffs. Recommended for new Drupal projects and teams familiar with Drupal coding standards.
* [AcquiaDrupalTransitional](src/Standards/AcquiaDrupalTransitional/ruleset.xml) incorporates AcquiaPHP and adds Drupal core's own phpcs configuration, which is less strict than the official standards. Recommended for legacy Drupal codebases or teams new to Drupal coding standards.
* [AcquiaPHP](src/Standards/AcquiaPHP/ruleset.xml) is the preferred standard for all Acquia projects, including Drupal projects. It is based on PSR-12 and compatible with Drupal standards. In other words, any code meeting the AcquiaPHP standard will also meet Drupal standards, but will take advantage of new language features offered by PSR-12 and (soon) PER-2.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the preferred standard for all Acquia projects, including Drupal projects

Did I misunderstand your proposal in Slack? This sounds like AcquiaPHP is, in fact, a superset of AcquiaDrupalStrict, i.e., based on and superseding it. In other words, we just flipped the direction of dependence so that AcquiaPHP is on the top now instead of the bottom; we didn't actually decouple them. I think that if I were a new user I would intuit the opposite based solely on their names--especially since that's what it meant before. (And, you know, only two hard problems. 🙂)

Maybe we could play around with some form of AcquiaPreferred or something, which I think implies more of our intent and doesn't carry the historical baggage? Or maybe we explore (what I thought was your original proposal) making AcquiaPHP compatible with AcquiaDrupal* but distinct--such that anyone could choose just one or the other if they wanted--and then create a new AcquiaPreferred that just references both AcquiaDrupalStrict and AcquiaPHP.

Copy link
Contributor

@TravisCarden TravisCarden Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is based on PSR-12 and compatible with Drupal standards.

Since there is actually some incompatibility between these two, this needs to be a "but" proposition. However we word it exactly, it needs to communicate that it's based on PSR-12* but is compatible with AcquiaDrupalStrict and where they conflict, AcquiaDrupalStrict wins.

*And does "based on" mean more like "inspired by" or "we included all of it except in cases of conflicts with Drupal standards"?

* [AcquiaDrupalStrict](src/Standards/AcquiaDrupalStrict/ruleset.xml) is based on Drupal coding standards and best practices sniffs. Recommended for legacy Drupal projects and teams familiar with Drupal coding standards that don't want to use the stricter AcquiaPHP standard.
* [AcquiaDrupalTransitional](src/Standards/AcquiaDrupalTransitional/ruleset.xml) is based on Drupal core's own phpcs configuration, which is less strict than the official standards. Recommended for legacy Drupal codebases or teams new to Drupal coding standards.
* [AcquiaEdge](src/Standards/AcquiaEdge/ruleset.xml) incorporates AcquiaPHP and adds backwards-incompatible sniffs that will be included in AcquiaPHP with the next major release of this package.

## Installation & usage
Expand Down
Loading
Loading