Remove PHPCS error as per Drupal standard:

phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md --ignore=node_modules,bower_components,vendor floating_block

FILE: ...dules/contrib/floating_block/src/Tests/FloatingBlockUnitTest.php
----------------------------------------------------------------------
FOUND 20 ERRORS AFFECTING 12 LINES
----------------------------------------------------------------------
3 | ERROR | [x] Namespaced classes, interfaces and traits should
| | not begin with a file doc comment
12 | ERROR | [x] Missing class doc comment
19 | ERROR | [x] Short array syntax must be used to define arrays
21 | ERROR | [x] Missing function doc comment
22 | ERROR | [x] Short array syntax must be used to define arrays
32 | ERROR | [x] Visibility must be declared on method "setUp"
40 | ERROR | [x] Visibility must be declared on method
| | "testBlockExampleBasic"
44 | ERROR | [x] Short array syntax must be used to define arrays
50 | ERROR | [x] Short array syntax must be used to define arrays
50 | ERROR | [x] Short array syntax must be used to define arrays
56 | ERROR | [x] Short array syntax must be used to define arrays
56 | ERROR | [x] Short array syntax must be used to define arrays
62 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
62 | ERROR | [x] Short array syntax must be used to define arrays
62 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
62 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
68 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...l/vimeo/modules/contrib/floating_block/src/Form/SettingsForm.php
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
3 | ERROR | [x] Namespaced classes, interfaces and traits should
| | not begin with a file doc comment
22 | ERROR | [x] Short array syntax must be used to define arrays
28 | ERROR | [ ] Public method name "SettingsForm::getFormID" is not
| | in lowerCamel format
37 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: .../vimeo/modules/contrib/floating_block/floating_block.routing.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: .../html/vimeo/modules/contrib/floating_block/floating_block.module
----------------------------------------------------------------------
FOUND 18 ERRORS AND 1 WARNING AFFECTING 18 LINES
----------------------------------------------------------------------
6 | ERROR | [ ] Doc comment short description must be on a
| | single line, further text should be a separate
| | paragraph
10 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
| | Implements hook_foo_BAR_ID_bar() for
| | xyz_bar().",, "* Implements
| | hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
| | "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.tpl.php.", or "* Implements
| | hook_foo_BAR_ID_bar() for block templates."
10 | ERROR | [x] Doc comment short description must end with a
| | full stop
25 | ERROR | [x] Namespaced classes/interfaces/traits should be
| | referenced with use statements
31 | ERROR | [x] Case breaking statements must be followed by a
| | single blank line
37 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
61 | ERROR | [x] Short array syntax must be used to define arrays
81 | ERROR | [ ] Type hint "array" missing for $floating_blocks
82 | ERROR | [x] Short array syntax must be used to define arrays
86 | ERROR | [x] Short array syntax must be used to define arrays
98 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
99 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
101 | ERROR | [x] Short array syntax must be used to define arrays
104 | ERROR | [x] Short array syntax must be used to define arrays
105 | ERROR | [x] Short array syntax must be used to define arrays
107 | ERROR | [x] Short array syntax must be used to define arrays
108 | ERROR | [x] Short array syntax must be used to define arrays
109 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 16 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 63ms; Memory: 6Mbphpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md --ignore=node_modules,bower_components,vendor floating_block

FILE: ...dules/contrib/floating_block/src/Tests/FloatingBlockUnitTest.php
----------------------------------------------------------------------
FOUND 20 ERRORS AFFECTING 12 LINES
----------------------------------------------------------------------
3 | ERROR | [x] Namespaced classes, interfaces and traits should
| | not begin with a file doc comment
12 | ERROR | [x] Missing class doc comment
19 | ERROR | [x] Short array syntax must be used to define arrays
21 | ERROR | [x] Missing function doc comment
22 | ERROR | [x] Short array syntax must be used to define arrays
32 | ERROR | [x] Visibility must be declared on method "setUp"
40 | ERROR | [x] Visibility must be declared on method
| | "testBlockExampleBasic"
44 | ERROR | [x] Short array syntax must be used to define arrays
50 | ERROR | [x] Short array syntax must be used to define arrays
50 | ERROR | [x] Short array syntax must be used to define arrays
56 | ERROR | [x] Short array syntax must be used to define arrays
56 | ERROR | [x] Short array syntax must be used to define arrays
62 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
62 | ERROR | [x] Short array syntax must be used to define arrays
62 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
62 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [ ] If the line declaring an array spans longer than 80
| | characters, each element should be broken into its
| | own line
68 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [x] Short array syntax must be used to define arrays
68 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 17 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...l/vimeo/modules/contrib/floating_block/src/Form/SettingsForm.php
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
3 | ERROR | [x] Namespaced classes, interfaces and traits should
| | not begin with a file doc comment
22 | ERROR | [x] Short array syntax must be used to define arrays
28 | ERROR | [ ] Public method name "SettingsForm::getFormID" is not
| | in lowerCamel format
37 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: .../vimeo/modules/contrib/floating_block/floating_block.routing.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: .../html/vimeo/modules/contrib/floating_block/floating_block.module
----------------------------------------------------------------------
FOUND 18 ERRORS AND 1 WARNING AFFECTING 18 LINES
----------------------------------------------------------------------
6 | ERROR | [ ] Doc comment short description must be on a
| | single line, further text should be a separate
| | paragraph
10 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
| | Implements hook_foo_BAR_ID_bar() for
| | xyz_bar().",, "* Implements
| | hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
| | "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.tpl.php.", or "* Implements
| | hook_foo_BAR_ID_bar() for block templates."
10 | ERROR | [x] Doc comment short description must end with a
| | full stop
25 | ERROR | [x] Namespaced classes/interfaces/traits should be
| | referenced with use statements
31 | ERROR | [x] Case breaking statements must be followed by a
| | single blank line
37 | ERROR | [x] Short array syntax must be used to define arrays
57 | ERROR | [x] Short array syntax must be used to define arrays
61 | ERROR | [x] Short array syntax must be used to define arrays
81 | ERROR | [ ] Type hint "array" missing for $floating_blocks
82 | ERROR | [x] Short array syntax must be used to define arrays
86 | ERROR | [x] Short array syntax must be used to define arrays
98 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
99 | ERROR | [x] Expected 1 space(s) before asterisk; 2 found
101 | ERROR | [x] Short array syntax must be used to define arrays
104 | ERROR | [x] Short array syntax must be used to define arrays
105 | ERROR | [x] Short array syntax must be used to define arrays
107 | ERROR | [x] Short array syntax must be used to define arrays
108 | ERROR | [x] Short array syntax must be used to define arrays
109 | ERROR | [x] Short array syntax must be used to define arrays
----------------------------------------------------------------------
PHPCBF CAN FIX THE 16 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 63ms; Memory: 6Mb

phpcs --standard=DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md --ignore=node_modules,bower_components,vendor floating_block

FILE: ...l/vimeo/modules/contrib/floating_block/src/Form/SettingsForm.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
39 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
41 | WARNING | t() calls should be avoided in classes, use
| | dependency injection and $this->t() instead
----------------------------------------------------------------------

FILE: ...tml/vimeo/modules/contrib/floating_block/floating_block.info.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
6 | WARNING | All dependencies must be prefixed with the project
| | name, for example "drupal:"
----------------------------------------------------------------------

Time: 27ms; Memory: 4Mb

CommentFileSizeAuthor
#12 floating_block-remove-phpcs-errors-2956970-12.patch4.2 KBmegachriz
#5 floating_block-remove_phpcs_errors-2956970-5-D8.patch11.3 KBkleiton_rodrigues
#2 fix-phpcs-error-2956970-2.patch12.23 KBneeraprajapati

Issue fork floating_block-2956970

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

neeraprajapati created an issue. See original summary.

neeraprajapati’s picture

neeraprajapati commented
Status: Active » Needs review
StatusFileSize
new fix-phpcs-error-2956970-2.patch12.23 KB

Applied patch for fixing phpcs errors.

neeraprajapati’s picture

neeraprajapati commented
Assigned: neeraprajapati » Unassigned
kleiton_rodrigues’s picture

kleiton_rodrigues commented
Assigned: Unassigned » kleiton_rodrigues
kleiton_rodrigues’s picture

kleiton_rodrigues commented
Assigned: kleiton_rodrigues » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new floating_block-remove_phpcs_errors-2956970-5-D8.patch11.3 KB

follows new patch

moving to RTBC

kleiton_rodrigues’s picture

kleiton_rodrigues commented
Assigned: Unassigned » kleiton_rodrigues
kleiton_rodrigues’s picture

kleiton_rodrigues commented
Assigned: kleiton_rodrigues » Unassigned
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Needs review

Please do not tag an issue patched by yourself as RTBC. It needs review of other users to become RTBC. Thanks for the hard work on it.

megachriz’s picture

megachriz
Primary language Dutch
commented

@diqidoq
It would be cool if automated tests are configured for the 8.x-1.x branch. Then we can see on the testbot if all code styles issues are indeed fixed with the patch.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented

DrupalCI testing is supported by Drupal Association members and supporting partners. See the DrupalCI documentation for more information. Testing costs $3-4k each month, so tests should be only configured on larger projects if really needed and should be considered carefully in respect for the whole community.

I rated Floating Block project in its size not as large enough in its whole scale and possible danger to break things in comparison to other projects, so I decided to keep the project out of the test environment.

But I will consider your recommendation and will communicate with the other maintainers about a possible compromise. Maybe weekly or monthly tests only.

EDIT: opened #3244396: [META] Test bot settings for the Floating Block project issue queue for tracking and opened D9 testing for the 8.x branch for now.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Needs review » Needs work

Needs a possible reroll against latest dev in the next days.

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs work » Postponed
StatusFileSize
new floating_block-remove-phpcs-errors-2956970-12.patch4.2 KB

Reroll. Depends on #3244563: Convert FloatingBlockUnitTest to PHPUnit unit test and move API functions to a service, therefore compared to the patch in #5, some CS fixes are no longer included in this one (it didn't make sense to me to add CS fixes to code that is removed by that other issue).

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Postponed » Needs work
Issue tags: +Needs reroll

Apparently, this needs a reroll again.

MegaChriz opened merge request !5

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll

Let's try that again, now using an issue fork so I can easily push more code if needed.

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Needs review » Needs work

Errrm... sorry @MegaChriz: I pushed already #2956921: Turn off functionality at small browser widths so this may have caused

d9/web/modules/contrib/floating_block$ git apply -v 5.diff
Checking patch floating_block.module...
error: while searching for:
<?php

use Drupal\Core\Routing\RouteMatchInterface;
/**
 * @file
 * The floating_block module allows parts of the site to stay on the page when
 * scrolling.
 */

/**
 * Implements hook_page_attachments();
 */
function floating_block_page_attachments(array &$attachments) {
  $floating_blocks = \Drupal::config('floating_block.settings')->get('blocks');

error: patch failed: floating_block.module:1
error: floating_block.module: patch does not apply
Checking patch src/Form/SettingsForm.php...
Hunk #1 succeeded at 93 (offset 11 lines).

I have seen the numbers of the diffs so maybe this here would have been better pushed before but I was OFF from the issue queue for 2 days and lost the overview. Would you mind to reroll one more time. It would take less time from your side where you are at than to revert commits and pushes etc. And I am not sure if it would break the chain from the other side then.

megachriz’s picture

megachriz
Primary language Dutch
commented

I could probably take a look at that tomorrow morning. :)

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs work » Needs review

I rebased the issue fork on the latest dev. There were no merge conflicts. Back to "Needs review".

dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Needs review » Reviewed & tested by the community

@MegaChriz: you rock! +1

/d9/web/modules/contrib/floating_block$ git apply -v 5.diff
Checking patch floating_block.module...
Checking patch src/Form/SettingsForm.php...
Applied patch floating_block.module cleanly.
Applied patch src/Form/SettingsForm.php cleanly.

Not broken functionality or error reports on a D8 and D9 test env found.

  • diqidoq committed a554c31 on 8.x-1.x authored by MegaChriz 
    Issue #2956970 by MegaChriz, neeraprajapati, kleiton_rodrigues, diqidoq...
dqd’s picture

dqd
Location London | N.Y.C | Paris | Hamburg | Berlin
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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