Problem/Motivation

Steps to reproduce

Run phpcs

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 drupal_coding_standard_fix-3266313-10.patch3.67 KBpriyanka_chauhan31
#7 field_tokens-3266313-7.patch4.69 KBsilvi.addweb
#6 field_tokens-phpcs-fixes-3266313-6.patch4.72 KBmrinalini9
#2 field_tokens-phpcs-fixes-3266313-2.patch4.91 KBbendale
field_tokens-phpcs-fixes.patch2.62 KBbendale

Issue fork field_tokens-3266313

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

bendale created an issue. See original summary.

bendale’s picture

bendale commented
StatusFileSize
new field_tokens-phpcs-fixes-3266313-2.patch4.91 KB

Found more phpcs fixes for the module. Updated the patch.

tmaiochi’s picture

tmaiochi commented
Assigned: Unassigned » tmaiochi

I'll review this!

tmaiochi’s picture

tmaiochi commented
Assigned: tmaiochi » Unassigned
Status: Active » Reviewed & tested by the community

Steps performed:
(1) Installed module
(2) Reproduced the issue.
(3) Applied patch.
(4) Code review on changes.
(5) Tested again with patch, issue resolved.
The patch fixed all PHPCS messages, moving to RTBC

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
Status: Reviewed & tested by the community » Needs work

Needs work as it will no longer apply since the automated d10 compatibility fix.

mrinalini9’s picture

mrinalini9 commented
Status: Needs work » Needs review
StatusFileSize
new field_tokens-phpcs-fixes-3266313-6.patch4.72 KB

Rerolled patch #2, please review it.

Thanks & Regards,
Mrinalini

silvi.addweb’s picture

silvi.addweb commented
StatusFileSize
new field_tokens-3266313-7.patch4.69 KB

Hello,
I looked over patch #6. Everything is operating as it should, although I discovered a new error, which I have resolved in the attached file.

FILE: /home/addweb/Drupal8-vagrant/web/web/patches/field_tokens/src/Tests/FieldTokensGeneralTest.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------
 6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\StringTranslation\StringTranslationTrait.
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------
clarkssquared’s picture

clarkssquared commented
Status: Needs review » Needs work

Hi @silvi

I applied the patch #7 and I confirmed that it fixes all the PHPCS issues.

➜  field_tokens git:(main) ✗ curl https://www.drupal.org/files/issues/2024-06-07/field_tokens-3266313-7.patch | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4804  100  4804    0     0  10286      0 --:--:-- --:--:-- --:--:-- 10489
patching file field_tokens.tokens.inc
patching file 'src/Tests/FieldTokensGeneralTest.php'
patching file 'src/Tests/FieldTokensTestBase.php'
➜  field_tokens git:(main) ✗ ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml field_tokens 

FILE: ...ts/d10/drupal_local/web/modules/contrib/field_tokens/field_tokens.info.yml
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by
   |         | drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by
   |         | drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by
   |         | drupal.org packaging automatically
--------------------------------------------------------------------------------

Time: 218ms; Memory: 12MB

➜  contrib git:(main) ✗

Can you make your patch into an MR? for the convenience of maintainers to merge it? I will transition this issue to needs work for that.

Thanks.

priyanka_chauhan31’s picture

priyanka_chauhan31 commented
Assigned: Unassigned » priyanka_chauhan31
priyanka_chauhan31’s picture

priyanka_chauhan31 commented
Assigned: priyanka_chauhan31 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new drupal_coding_standard_fix-3266313-10.patch3.67 KB

Please review the patch.

liam morland made their first commit to this issue’s fork.

liam morland opened merge request !5

paraderojether’s picture

paraderojether commented
Status: Needs review » Reviewed & tested by the community

Hi

I reviewed MR!5 and confirmed phpcs and cspell are passed on Gitlab CI.

Moving this to RBTC!
Thank you.

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
Version: 2.0.0-rc1 » 2.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 2.0.x branch as part of the D11 compatibility merge. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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