-
Notifications
You must be signed in to change notification settings - Fork 461
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.
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
introduce phpcs and conform to psr12 #6600
Conversation
Should be fine (and easier) to go directly to psr12 |
Couple of observations from my experience at Joomla when we moved from an internal coding standard to psr-12 We did this in two different pull requests. The first was to set up the tooling and the second was to use the tooling to make the change. If you want to take a look you can see that in the Joomla build folder there are the conversion scripts /~https://github.com/joomla/joomla-cms/tree/5.0-dev/build/psr12 The second part is to ensure that future code is compliant and that developers can run the checks locally and not have to rely on a github action. For this we have a ruleset.xml in the root and have phpcs installed with composer as a dev dependency |
I chose to do it in one PR because it doesn't really make sense to separate out these efforts when one depends on another. |
Thank you again for doing this. This PR is so big I'm unable to process it all. Also there are changes to the backup which seems to be not part of this cleanup I think I'll see if I can create smaller PRs for my ability to understand and test |
Good news I can use the commits to cherry pick into master |
BackupManager splitting into file-per-class is part of the same cleanup effort. |
I didnt see the local part |
Yup, this was added to |
I was looking for something more explicit and obvious |
What does PR do? Any background context you want to provide?
code style was all over the place so I added phpcs for code linting - I will add more rules later but this change was already getting massive.
What Issues does it Close? Any the relevant tickets?
Screenshots (if appropriate)
Where should the reviewer start?
this might be hard to review...
How should this be manually tested?
regression test
Questions:
Is there a related website / article to substantiate / explain this change?
Does the development wiki need an update?
Does the user documentation wiki need an update?
Does this add new dependencies?
Does this need to add new data to the demo database