[#39197] - [4.3] Convert missing files to PSR-12 code style
- Fixed in Code Base
- 27 Jan 2023
- Medium
- Build: 4.2-dev
- # 39197
- Diff
- heelc29:patch-psr12
User tests: Successful: Unsuccessful:
Pull Request for Issue #38754 and #38786 .
Summary of Changes
Note that not all files were converted to the PSR-12 codestyle during PR #37686 eg:
- administrator/components/com_actionlogs/src/View/Action
logs/HtmlView.php - administrator/components/com_admin/src/Service/HTML/
Configuration.php - administrator/components/com_cache/tmpl/
cache/default.php - administrator/components/com_media/tmpl/
media/default.php - api/components/com_
media/src/Model/MediaModel.php - layouts/joomla/form/field/
media/accessiblemedia.php - libraries/src/
Cache/Cache.php - libraries/src/Component/Router/RouterView
Configuration.php - plugins/fields/
media/media.php - plugins/privacy/action
logs/actionlogs.php - plugins/system/
cache/src/Extension/Cache.php - plugins/webservices/
media/media.php
These files are therefore not checked by Drone now. This is due to an overly open exclude list (marked in list above).
Testing Instructions
- Drone works
- Code Review
- Install Joomla
Actual result BEFORE applying this Pull Request
2648 files are checked by phpcs
Expected result AFTER applying this Pull Request
2743 files are checked by phpcs
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
cc @HLeithner
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin com_cache com_media Repository Layout Libraries |
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39197.
Regarding the testing instructions: "Install Joomla" is not necessary
Just copied from PR #37686 - but only a part of it
But it could make sense to let people run the PHPCS tests locally in their environment
This is not always possible because I have problems with the EOL in my win environment:
| Labels |
Added:
?
|
||
@heelc29 Why have you updated your branch without any need? It resets the counter for human tests and I manually have to add the test result again in the issue tracker. That causes unnecessary work and may cause the one or other PR with 2 good test results being overlooked.
@heelc29 there is usually a setting in your git client that will resolve that
@brianteeman I use GitHub Desktop with default settings ... mmmh so I can't be sure that everyone else has changed it at their git client
@heelc29 Why have you updated your branch without any need? It resets the counter for human tests and I manually have to add the test result again in the issue tracker. That causes unnecessary work and may cause the one or other PR with 2 good test results being overlooked.
@richard67 I thought to make sure there are no conflicts or new code style errors from the changed ruleset.
. mmmh so I can't be sure that everyone else has changed it at their git client
Thats irrelevant. Just follow https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings
| Labels |
Added:
Conflicting Files
|
||
Conflicts resolved in:
- administrator/components/com_actionlogs/tmpl/actionlogs/default.php
- libraries/src/Cache/Cache.php
- libraries/src/Cache/CacheController.php
- libraries/src/Cache/CacheStorage.php
- libraries/src/Cache/Controller/CallbackController.php
- libraries/src/Cache/Storage/FileStorage.php
- libraries/src/Cache/Storage/MemcachedStorage.php
- libraries/src/Cache/Storage/RedisStorage.php
- plugins/privacy/actionlogs/actionlogs.php
- plugins/system/actionlogs/actionlogs.php
sorry that this took so long, can you change the target branch to 4.3 pleasE?
@HLeithner Sure, but I will wait for the next upmerge, there are many conflicts at the moment:
[4.3] Change application input access to getInput #39029 vs. Changes the code base to short array syntax #39616
And a tricky one too in libraries/namespacemap.php which I don't want to resolve. Take care here @obuisard ;)
[4.2] Fix library autoloading #39348 vs. Add namespace support to templates #39011
| Category | Administration com_admin com_cache com_media Repository Layout Libraries | ⇒ | Unit Tests Repository Administration com_admin SQL Postgresql com_associations com_banners |
| Labels |
Removed:
Conflicting Files
|
||
sorry that this took so long, can you change the target branch to 4.3 pleasE?
@HLeithner done
| Category | Administration com_admin Repository Unit Tests SQL Postgresql com_associations com_banners | ⇒ | Administration com_admin com_cache com_media Repository Layout Libraries |
| Title |
|
||||||
| Labels |
Added:
PR-4.3-dev
Removed: ? |
||
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-01-27 14:15:11 |
| Closed_By | ⇒ | HLeithner |
@heelc29 Regarding the testing instructions: "Install Joomla" is not necessary. That is included already in the "Drone works". The API and system tests include making a new installation on diverse environments, so if these pass, installation works. But it could make sense to let people run the PHPCS tests locally in their environment with
./libraries/vendor/bin/phpcs --extensions=php -p --standard=ruleset.xml .and check that this works.