CommentFileSizeAuthor
#5 3153141-5.patch19.74 KBPooja Ganjage
#2 3153141-2.patch15.63 KBkarishmaamin

Issue fork owlcarousel-3153141

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

karishmaamin created an issue. See original summary.

karishmaamin’s picture

karishmaamin CreditAttribution: karishmaamin commented
Status: Active » Needs review
FileSize
3153141-2.patch15.63 KB

Status: Needs review » Needs work

The last submitted patch, 2: 3153141-2.patch, failed testing. View results

guilhermevp’s picture

guilhermevp CreditAttribution: guilhermevp at CI&T commented

The patch address most of the issues but PHPCS still complaining about some stuff: 

 $ phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml owl-carousel/

FILE: /home/guilhermevp/Ambientes/Drupal 9.2.x/drupal/modules/owl-carousel/README.md
------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------
 11 | WARNING | Line exceeds 80 characters; contains 274 characters
------------------------------------------------------------------------------------


FILE: /home/guilhermevp/Ambientes/Drupal 9.2.x/drupal/modules/owl-carousel/src/OwlCarouselGlobal.php
----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------
 7 | WARNING | The class short comment should describe what the class does and not simply repeat the class name
----------------------------------------------------------------------------------------------------------------


FILE: /home/guilhermevp/Ambientes/Drupal 9.2.x/drupal/modules/owl-carousel/src/Plugin/Field/FieldFormatter/OwlCarouselFieldFormatter.php
----------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------
 288 | WARNING | [x] 'TODO: Implement settings summary.' should match the format '@todo Fix problem X here.'
----------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------

Time: 192ms; Memory: 10MB
Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
FileSize
3153141-5.patch19.74 KB

Hi,

Creating a patch as suggested in #4 comment.

Please review the patch.

Thanks.

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: 3153141-5.patch, failed testing. View results

Pooja Ganjage’s picture

Pooja Ganjage CreditAttribution: Pooja Ganjage at Asentech LLC commented
Status: Needs work » Needs review
ipwa’s picture

ipwa CreditAttribution: ipwa at manifesto commented
Status: Needs review » Needs work

This patch needs to be rerolled it does not apply to the current 8.x-1.x branch

chakkche’s picture

chakkche CreditAttribution: chakkche as a volunteer and at QED42 commented
Status: Needs work » Reviewed & tested by the community

@ipwa currently i didn't find any coding standard issues in the latest branch. So moving it to RTBC. May be we can move it to closed(work as designed state).

ipwa’s picture

ipwa CreditAttribution: ipwa at manifesto commented
Status: Reviewed & tested by the community » Needs work

Yes you are right @chakkche no coding standard issues on the 8.x-2.x branch I think largely thanks to you!

However this issue is for the 8.x-1.x branch so if someone want to reroll the patch from @Pooja Ganjage then I would commit it to the 8.x-1.x branch.

chakkche’s picture

chakkche CreditAttribution: chakkche as a volunteer and at QED42 commented
Assigned: Unassigned » chakkche

ok then i will work on this issue

chakkche opened merge request !5

chakkche’s picture

chakkche CreditAttribution: chakkche as a volunteer and at QED42 commented
Assigned: chakkche » Unassigned
Status: Needs work » Needs review

@ipwa Fixed the coding standards issues in 8.x-1.x branch

  • ipwa committed a636e57 on 8.x-1.x authored by chakkche 
    Resolve #3153141 "Fix coding standards"
ipwa’s picture

ipwa CreditAttribution: ipwa at manifesto commented

Thanks so much @chakkche for the merge request, also thanks to @Pooja Ganjage and @karishmaamin for original patches.

ipwa’s picture

ipwa CreditAttribution: ipwa at manifesto commented
Status: Needs review » Fixed

Also thanks to @guilhermevp for testing!

Status: Fixed » Closed (fixed)

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