More information at this link https://git.drupalcode.org/issue/site_settings-3398115/-/jobs/256577

Issue fork site_settings-3398369

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bobi-mel created an issue. See original summary.

bobi-mel opened merge request !14

bobi-mel’s picture

bobi-mel commented
Assigned: bobi-mel » Unassigned
Status: Active » Needs review

I fixed PHPCS

bobi-mel’s picture

bobi-mel commented
Issue summary: View changes
clarkssquared’s picture

clarkssquared commented
Status: Needs review » Needs work

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) ✗
bobi-mel’s picture

bobi-mel commented
Assigned: Unassigned » bobi-mel
bobi-mel’s picture

bobi-mel commented
Assigned: bobi-mel » Unassigned
Status: Needs work » Needs review

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.

scott_euser made their first commit to this issue’s fork.

scott_euser’s picture

scott_euser commented
Status: Needs review » Fixed

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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

ekes’s picture

ekes commented

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.

ekes’s picture

ekes commented

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 :)

scott_euser opened merge request !20

scott_euser’s picture

scott_euser commented

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

ekes’s picture

ekes commented

I stumbled on the fatal error when bumping 8.x-1.x up to HEAD (actually to get the fix for #3410320: Routing error with Drupal 10.2 (Route "entity.site_setting_entity_type.canonical" does not exist) / #3357022: Support "Clone" functionality). So yes I was fixing 8.x-1.x . l hadn't checked 2.x sorry.

nagy.balint’s picture

nagy.balint commented

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 TypeError

It might be easier to just open a new issue, and push this 1 line commit, unless the 1.x branch is discontinued soon.

nagy.balint’s picture

nagy.balint commented

I created #3420953: TypeError in admin/content/site-settings for that purpose.

scott_euser’s picture

scott_euser commented

Thanks! Merged #3420953: TypeError in admin/content/site-settings to sort the issue, much appreciated @nagy.balint