- Issue created by @Bobik
- last update
9 months ago 2 pass - @bobi-mel opened merge request.
- Issue was unassigned.
- Status changed to Needs review
9 months ago 2:49pm 1 November 2023 - Status changed to Needs work
9 months ago 5:18am 3 November 2023 - 🇵🇭Philippines clarkssquared
Hi bobi,
I applied your MR !14 and there are still many PHPCS issues being flagged, hence moving this to needs work
➜ site_settings git:(e8677ad) curl https://git.drupalcode.org/project/site_settings/-/merge_requests/14.diff | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 3527 0 3527 0 0 7425 0 --:--:-- --:--:-- --:--:-- 7504 patching file 'modules/site_settings_type_permissions/site_settings_type_permissions.module' patching file site_settings.install patching file 'src/Plugin/Block/RenderedSiteSettingsBlock.php' patching file 'src/Plugin/Block/SimpleSiteSettingsBlock.php' patching file 'tests/modules/site_settings_sample_data/site_settings_sample_data.module' ➜ site_settings git:(e8677ad) ✗ .. ➜ contrib git:(master) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml site_settings FILE: ...d9/d9-local/web/modules/contrib/site_settings/site_settings.links.task.yml -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 21 | ERROR | [x] Expected 1 newline at end of file; 0 found -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: .../d9-local/web/modules/contrib/site_settings/site_settings.links.action.yml -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | ERROR | [x] Expected 1 newline at end of file; 2 found -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- FILE: ...ts/modules/site_settings_sample_data/site_settings_sample_data.routing.yml -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 8 | WARNING | Open page callback found, please add a comment before the line | | why there is no access restriction -------------------------------------------------------------------------------- FILE: .../tests/modules/site_settings_sample_data/site_settings_sample_data.install -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 143 | WARNING | Redeclaration of global variable $base_url as global variable. -------------------------------------------------------------------------------- FILE: ...odules/site_settings_sample_data/src/Controller/TestSiteSettingsLoader.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 26 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead -------------------------------------------------------------------------------- FILE: ...ng-subing/Projects/d9/d9-local/web/modules/contrib/site_settings/README.md -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES -------------------------------------------------------------------------------- 44 | WARNING | Line exceeds 80 characters; contains 90 characters 45 | WARNING | Line exceeds 80 characters; contains 100 characters 61 | WARNING | Line exceeds 80 characters; contains 111 characters 62 | WARNING | Line exceeds 80 characters; contains 114 characters 73 | WARNING | Line exceeds 80 characters; contains 82 characters -------------------------------------------------------------------------------- FILE: ...d9/d9-local/web/modules/contrib/site_settings/site_setting_entity.page.inc -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 26 | WARNING | Unused variable $site_setting_entity. -------------------------------------------------------------------------------- FILE: ...cts/d9/d9-local/web/modules/contrib/site_settings/site_settings.tokens.inc -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 105 | WARNING | Unused variable $token_service. -------------------------------------------------------------------------------- FILE: .../d9-local/web/modules/contrib/site_settings/src/SiteSettingsReplicator.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 6 WARNINGS AFFECTING 6 LINES -------------------------------------------------------------------------------- 96 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 97 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 98 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 138 | WARNING | Unused variable $key. 196 | WARNING | Unused variable $bundle. 237 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead -------------------------------------------------------------------------------- FILE: ...al/web/modules/contrib/site_settings/src/Form/SiteSettingReplicateForm.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES -------------------------------------------------------------------------------- 167 | WARNING | Unused variable $existing. 206 | WARNING | Unused variable $key. 259 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead -------------------------------------------------------------------------------- FILE: ...ocal/web/modules/contrib/site_settings/src/Form/SiteSettingsConfigForm.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES -------------------------------------------------------------------------------- 83 | WARNING | t() calls should be avoided in classes, use | | \Drupal\Core\StringTranslation\StringTranslationTrait and | | $this->t() instead 84 | WARNING | t() calls should be avoided in classes, use | | \Drupal\Core\StringTranslation\StringTranslationTrait and | | $this->t() instead -------------------------------------------------------------------------------- FILE: ...s/d9/d9-local/web/modules/contrib/site_settings/src/SiteSettingsLoader.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES -------------------------------------------------------------------------------- 75 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 89 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 96 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead -------------------------------------------------------------------------------- FILE: ...cal/web/modules/contrib/site_settings/src/SiteSettingEntityListBuilder.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 145 | WARNING | Unused variable $key. -------------------------------------------------------------------------------- FILE: ...dules/contrib/site_settings/src/Plugin/Block/RenderedSiteSettingsBlock.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 131 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead -------------------------------------------------------------------------------- FILE: ...local/web/modules/contrib/site_settings/src/SiteSettingEntityInterface.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------------- 16 | WARNING | There must be no blank line following an inline comment -------------------------------------------------------------------------------- Time: 1.11 secs; Memory: 12MB ➜ contrib git:(master) ✗ - Assigned to Bobik
- Issue was unassigned.
- Status changed to Needs review
9 months ago 2:50pm 3 November 2023 - 🇺🇦Ukraine Bobik
Hi Clarkssquared!
I fixed the errors detected by the PHPCS job indicated in the link. Also @scott_euser preparing a major update for the module and most of these issues will be fixed or obsolete. Summarizing the above, I believe that there is no point in fixing something additionally, as this will lead to complications when merging branches. - First commit to issue fork.
-
scott_euser →
committed 1bf2cdc1 on 8.x-1.x authored by
bobi-mel →
Issue #3398369 by bobi-mel, clarkssquared: Fix PHPCS
-
scott_euser →
committed 1bf2cdc1 on 8.x-1.x authored by
bobi-mel →
- Status changed to Fixed
9 months ago 7:13am 8 November 2023 - 🇬🇧United Kingdom scott_euser
Thanks! Like bobi-mel said, we are mostly moved on to the 2.x branch, but does not hurt to merge this into 8.x-1.x of course!
Automatically closed - issue fixed for 2 weeks with no activity.
- Status changed to Fixed
6 months ago 1:20pm 15 January 2024 - 🇳🇱Netherlands ekes
I think that last patch got a bit over enthusiastic https://git.drupalcode.org/project/site_settings/-/commit/1bf2cdc17efe24... is still in use, but removed.
I'll add it to the (already merged) branch, and it could be merged again.
- 🇳🇱Netherlands ekes
Meh. I can't reopen the issue, but it doesn't seem worth creating a new one for https://git.drupalcode.org/issue/site_settings-3398369/-/commit/696253fc... this. Hopefully you'll notice :)
- 🇬🇧United Kingdom scott_euser
Thanks @ekes! I re-opened it here, but I think its quite a bit behind (or is the intention to keep this targeting 8x-1x rather than 2x branch?)
https://git.drupalcode.org/project/site_settings/-/merge_requests/20 - 🇳🇱Netherlands ekes
I stumbled on the fatal error when bumping 8.x-1.x up to HEAD (actually to get the fix for 🐛 Routing error with Drupal 10.2 (Route "entity.site_setting_entity_type.canonical" does not exist) Closed: duplicate / ✨ Support "Clone" functionality Fixed ). So yes I was fixing 8.x-1.x . l hadn't checked 2.x sorry.
- 🇭🇺Hungary nagy.balint
Yes currently 1.x is broken due to the commit here since "use Drupal\Core\Session\AccountInterface;" was removed.
This caused admin/content/site-settings to throw a TypeErrorIt might be easier to just open a new issue, and push this 1 line commit, unless the 1.x branch is discontinued soon.
- 🇭🇺Hungary nagy.balint
I created 🐛 TypeError in admin/content/site-settings Fixed for that purpose.
- 🇬🇧United Kingdom scott_euser
Thanks! Merged 🐛 TypeError in admin/content/site-settings Fixed to sort the issue, much appreciated @nagy.balint