Problem/Motivation

Need to resolve below Drupal coding standard issues.

FILE: /Users/arunkumar/Contribution/domain_entity/tests/src/Functional/DomainEntitySourceUrlsTest.php
--------------------------------------------------------------------------------------------------------------------------------------------
FOUND 12 ERRORS AND 3 WARNINGS AFFECTING 15 LINES
--------------------------------------------------------------------------------------------------------------------------------------------
  20 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
  36 | WARNING | [x] A comma should follow the last multiline array item. Found: 'taxonomy'
  63 | ERROR   | [ ] Missing short description in doc comment
  66 | ERROR   | [ ] Class property $source_mapper should use lowerCamel naming without underscores
  68 | ERROR   | [ ] Missing short description in doc comment
  71 | ERROR   | [ ] Class property $domain_storage should use lowerCamel naming without underscores
  73 | ERROR   | [ ] Missing short description in doc comment
  76 | ERROR   | [ ] Class property $domain_source_id should use lowerCamel naming without underscores
  95 | ERROR   | [ ] The array declaration extends to column 93 (the limit is 80). The array content should be split up over multiple lines
 104 | ERROR   | [ ] Missing parameter comment
 106 | ERROR   | [ ] Description for the @return value is missing
 115 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
 140 | ERROR   | [x] Expected 1 blank line after function; 2 found
 144 | WARNING | [ ] Line exceeds 80 characters; contains 82 characters
 186 | ERROR   | [x] Missing function doc comment
--------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/HttpKernel/DomainEntitySourcePathProcessor.php
----------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
----------------------------------------------------------------------------------------------------
  93 | ERROR   | [ ] Missing short description in doc comment
 110 | ERROR   | [x] Parameter comment must end with a full stop
 147 | ERROR   | [x] Perl-style comments are not allowed; use "// Comment" instead
 198 | ERROR   | [x] Expected 1 blank line after function; 2 found
 270 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/Form/DomainEntityUi.php
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------
 109 | WARNING | [x] '@Todo Port active domain UI effects.' should match the format '@todo Fix problem X here.'
----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/Form/DomainEntitySettings.php
--------------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
--------------------------------------------------------------------------------------
  64 | ERROR   | [ ] Parameter $source_mapper is not described in comment
 185 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 226 | ERROR   | [x] Array closing indentation error, expected 10 spaces but found 12
--------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/DomainEntitySourceMapper.php
--------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------
 34 | WARNING | [x] '@Todo Add better naming for entities without bundles.' should match the format '@todo Fix problem X here.'
--------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/DomainEntityMapper.php
---------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------
 144 | WARNING | [x] '@Todo Add better naming for entities without bundles.' should match the format '@todo Fix problem X here.'
---------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/DomainEntityPermissions.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 56 | ERROR | The array declaration extends to column 166 (the limit is 80). The array content should be split up over multiple lines
 59 | ERROR | The array declaration extends to column 164 (the limit is 80). The array content should be split up over multiple lines
 62 | ERROR | The array declaration extends to column 166 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/arunkumar/Contribution/domain_entity/src/EventSubscriber/DomainEntitySourceRedirectResponseSubscriber.php
----------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 54 | ERROR | [x] Expected newline after closing brace
----------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------

Time: 206ms; Memory: 10MB

Steps to reproduce

phpcs --standard=Drupal domain_entity
phpcs --standard=DrupalPractice domain_entity

Proposed resolution

Resolve all coding standard issues.

Remaining tasks

Nil

User interface changes

Nil

API changes

Nil

Data model changes

Nil

CommentFileSizeAuthor
#27 interdiff_24-27.txt22.24 KBjatingupta40
#27 updated-3295668-27.patch23.96 KBjatingupta40
#26 Screenshot from 2022-07-25 18-15-35.png44.44 KBjatingupta40
#24 interdiff_21-24.txt2.18 KBalanmoreira
#24 3295668-24.patch23.37 KBalanmoreira
#22 interdiff_15-21.txt1.28 KBalanmoreira
#22 3295668-21.patch24.15 KBalanmoreira
#19 Screenshot from 2022-07-25 10-11-28.png104.82 KBnitin_lama
#15 interdiff_10-15.txt429 bytessourabhjain
#15 3295668-15.patch17.48 KBsourabhjain
#10 interdiff_8-10.txt476 bytessourabhjain
#10 3295668-10.patch17.49 KBsourabhjain
#8 interdiff_3-8.txt4.26 KBmrinalini9
#8 Coding_standard-3295668-8.patch17.17 KBmrinalini9
#6 Screen Shot 2022-07-19 at 03.29.31.png1.08 MBdelta
#3 Coding_standard-3295668-3.patch14.56 KBatul_ghate

Issue fork domain_entity-3295668

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

arunkumark created an issue. See original summary.

atul_ghate’s picture

atul_ghate commented
Assigned: Unassigned » atul_ghate

Hi.
I will work on that issue

atul_ghate’s picture

atul_ghate commented
Assigned: atul_ghate » Unassigned
Status: Active » Needs review
StatusFileSize
new Coding_standard-3295668-3.patch14.56 KB

Hello,
Please review the patch I have created for the above mentioned coding standard issues.

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

I will review this patch.

jatingupta40’s picture

jatingupta40 commented
Assigned: jatingupta40 » Unassigned
Status: Needs review » Reviewed & tested by the community

The patch is working absolutely fine. No remaining errors.
Thanks

delta’s picture

delta commented
StatusFileSize
new Screen Shot 2022-07-19 at 03.29.31.png1.08 MB

quick glance, php docs are updated, not variables
not sure whats the point here
but if you want to finalize that patch I'm happy to merge it when its ready

delta’s picture

delta commented
Status: Reviewed & tested by the community » Needs work
mrinalini9’s picture

mrinalini9 commented
Status: Needs work » Needs review
StatusFileSize
new Coding_standard-3295668-8.patch17.17 KB
new interdiff_3-8.txt4.26 KB

Updated patch #3 by addressing #6, please review it.

sourabhjain’s picture

sourabhjain
Primary language English
commented
Assigned: Unassigned » sourabhjain

I will review it.

sourabhjain’s picture

sourabhjain
Primary language English
commented
Assigned: sourabhjain » Unassigned
StatusFileSize
new 3295668-10.patch17.49 KB
new interdiff_8-10.txt476 bytes

I have reviewed the patch and found the below errors:

FILE: /Users/sourabhjain/www/2022/drupal/modules/contrib/domain_entity/src/HttpKernel/DomainEntitySourcePathProcessor.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------
 167 | ERROR | [x] Expected newline after closing brace
 168 | ERROR | [x] No space found before comment text; expected "// throw $th;" but found "//throw $th;"
 168 | ERROR | [x] Inline comments must start with a capital letter
 168 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

Fixed the above issues. Please review.

Shubham Sharma 77’s picture

Shubham Sharma 77 commented
Status: Needs review » Reviewed & tested by the community

Coding standard issues have been resolved. Move this ticket to RTBC.

jatingupta40’s picture

jatingupta40 commented

@sourabhjain
Why this extra semicolon is there ? Is it needed ?

+++ b/domain_entity.module
@@ -74,7 +74,8 @@ function domain_entity_form_field_storage_config_edit_form_alter(&$form, FormSta
+    ;
delta’s picture

delta commented
Status: Reviewed & tested by the community » Needs work

see #12

and snake case variables doesn't need to be changed to camel case, see https://www.drupal.org/node/318#s-functions-and-variables

nitin_lama’s picture

nitin_lama
he/him
Location India
commented

@JatinGupta40 the extra semicolon is being removed by the patch #10.

sourabhjain’s picture

sourabhjain
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 3295668-15.patch17.48 KB
new interdiff_10-15.txt429 bytes

Removed the extra semicolon as mentioned in #12.

shamir.vs’s picture

shamir.vs commented
Status: Needs review » Reviewed & tested by the community

Patch #15 applied successfully and all coding standard issue is fixed

jatingupta40’s picture

jatingupta40 commented

@nitin_lama Thanks for replying.
@sourabhjain Thanks for replying and making the correction.

Regards.

delta’s picture

delta commented
Status: Reviewed & tested by the community » Needs work

https://www.drupal.org/node/318#s-functions-and-variables

Variables should be named using lowercase, and words should be separated either with uppercase characters (example: $lowerCamelCase) or with an underscore (example: $snake_case). Be consistent; do not mix camelCase and snake_case variable naming inside a file.

do not mix camelCase and snake_case variable naming inside a file.

there is no reason to change the case style.

The patch change some variable to camelCase and break coding standards by mixing case.

-  public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityTypeBundleInfoInterface $entity_type_bundle_info, DomainEntityMapper $mapper, DomainEntitySourceMapper $source_mapper) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityTypeBundleInfoInterface $entity_type_bundle_info, DomainEntityMapper $mapper, DomainEntitySourceMapper $sourceMapper) {

please revert the case style changes.

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
StatusFileSize
new Screenshot from 2022-07-25 10-11-28.png104.82 KB

Patch failed to apply. #15
failed_patch

alanmoreira’s picture

alanmoreira commented
Assigned: Unassigned » alanmoreira

I'll try to re-roll patch #15

jatingupta40’s picture

jatingupta40 commented

@alanmoreira Please also try to solve the issue suggested in comment #18
Thanks and regards.

alanmoreira’s picture

alanmoreira commented
Assigned: alanmoreira » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3295668-21.patch24.15 KB
new interdiff_15-21.txt1.28 KB

I rerolled the patch and fixed the remaining phpcs errors. Changing status to "Needs review" =)

JatinGupta40 opened merge request !9

alanmoreira’s picture

alanmoreira commented
StatusFileSize
new 3295668-24.patch23.37 KB
new interdiff_21-24.txt2.18 KB

@JatinGupta40 , I just saw your comment right after i posted the #22 comment. Fixing what is asked in #18. Please review it :)

jatingupta40’s picture

jatingupta40 commented
Assigned: Unassigned » jatingupta40

@alanmoreira
Thanks for the reply, i will review you patch.

jatingupta40’s picture

jatingupta40 commented
StatusFileSize
new Screenshot from 2022-07-25 18-15-35.png44.44 KB

While reviewing the patch provided at #24, it resolves all the suggestion suggested in comment #18, but got 2 coding standard issues.
Working on it will soon provide an updated patch.
Thanks

jatingupta40’s picture

jatingupta40 commented
Assigned: jatingupta40 » Unassigned
StatusFileSize
new updated-3295668-27.patch23.96 KB
new interdiff_24-27.txt22.24 KB

Providing an updated patch.
Please review.
Thanks.

atul_ghate’s picture

atul_ghate commented
Assigned: Unassigned » atul_ghate

Hi,
i will review this patch.

atul_ghate’s picture

atul_ghate commented
Assigned: atul_ghate » Unassigned
Status: Needs review » Reviewed & tested by the community

#27 resolves all coding standards issues and patch applies cleanly, moving to RTBC.

  • delta committed 74524f6 on 8.x-1.x 
    Issue #3295668 by atul ghate,JatinGupta40,mrinalini9,sourabhjain,Shubham...
delta’s picture

delta commented
Status: Reviewed & tested by the community » Fixed

thank you all

sourabhjain’s picture

sourabhjain
Primary language English
commented

Status: Fixed » Closed (fixed)

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

bohart closed merge request !9