Running phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig for the 2.0.x branch shows the following warnings/errors that should be fixed.

FILE: ./modules/single_page_site_next_page/src/EventSubscriber/AlterSinglePageSiteOutput.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 16 | WARNING | The class short comment should describe what the
    |         | class does and not simply repeat the class name
----------------------------------------------------------------------


FILE: ./single_page_site.module
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 10 | ERROR | [x] Use statements should be sorted alphabetically. The
    |       |     first wrong one is
    |       |     Drupal\Core\Form\FormStateInterface.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Controller/SinglePageSiteController.php
----------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Controller\ControllerBase.
  20 | WARNING | [ ] The class short comment should describe what the
     |         |     class does and not simply repeat the class name
  96 | WARNING | [ ] Unused variable $menu.
 178 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Event/EventSinglePageSiteAlterOutput.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 11 | WARNING | The class short comment should describe what the
    |         | class does and not simply repeat the class name
----------------------------------------------------------------------


FILE: ./src/Event/SinglePageSiteEvents.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 9 | WARNING | The class short comment should describe what the class
   |         | does and not simply repeat the class name
----------------------------------------------------------------------


FILE: ./src/Form/SinglePageSiteConfigForm.php
----------------------------------------------------------------------
FOUND 3 ERRORS AND 31 WARNINGS AFFECTING 34 LINES
----------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically.
     |         |     The first wrong one is
     |         |     Drupal\Core\Form\ConfigFormBase.
  15 | WARNING | [ ] The class short comment should describe what the
     |         |     class does and not simply repeat the class name
  71 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  76 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  81 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  85 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  89 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
  98 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 107 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 111 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 115 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 119 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 122 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 123 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 126 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 128 | ERROR   | [x] Use null coalesce operator instead of ternary
     |         |     operator.
 133 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 138 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 142 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 146 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 150 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 155 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 161 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 162 | ERROR   | [x] Use null coalesce operator instead of ternary
     |         |     operator.
 167 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 172 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 175 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 179 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 182 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 198 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 205 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 211 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 241 | WARNING | [ ] t() calls should be avoided in classes, use
     |         |     \Drupal\Core\StringTranslation\StringTranslationTrait
     |         |     and $this->t() instead
 251 | WARNING | [ ] \Drupal calls should be avoided in classes, use
     |         |     dependency injection instead
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ./src/Manager/SinglePageSiteManager.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 27 | WARNING | The class short comment should describe what the
    |         | class does and not simply repeat the class name
----------------------------------------------------------------------

Time: 182ms; Memory: 10MB
CommentFileSizeAuthor
#33 Enabled_Gitlab_CI.png554.4 KBroberttabigue
#22 phpcsafterpatch.png74.79 KBroberttabigue
#22 phpcsbeforepatch.png774.4 KBroberttabigue
#20 update.patch14.84 KBanjali mehta
#19 fixed.patch2.13 KBanjali mehta
#17 After-patch.png118.71 KBanjali mehta
#17 Before-patch.png145.24 KBanjali mehta
#16 3150482-16.patch13.71 KBbharath-kondeti
#7 phpcs-issue-fixes-3150482-7.patch32.24 KBashutosh ahirwal
#4 3150482-4.patch32.82 KBvlklavanya
#3 3150482-3.patch22.63 KBpavnish

Issue fork single_page_site-3150482

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

pavnish created an issue. See original summary.

pavnish’s picture

pavnish commented
Issue summary: View changes
pavnish’s picture

pavnish commented
Assigned: pavnish » Unassigned
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3150482-3.patch22.63 KB

Please review this patch

vlklavanya’s picture

vlklavanya commented
StatusFileSize
new 3150482-4.patch32.82 KB

Removed t() calls and resolved dependency injection in this patch.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Category: Bug report » Task
Priority: Normal » Minor
Status: Needs review » Needs work
Issue tags: +Coding standards, +Needs issue summary update

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Drupal Coding standard issue » Fix the issues reported by phpcs
Issue tags: -Drupal 9 compatibility
ashutosh ahirwal’s picture

ashutosh ahirwal commented
Status: Needs work » Needs review
StatusFileSize
new phpcs-issue-fixes-3150482-7.patch32.24 KB

Providing updated patch with phpcs issue fixes.
please review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

The issue summary must still be updated.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

Also, the report shows warnings/errors for 13 files; the last patch changes 17 files. Either the report in the issue summary is not complete or the patch changes more files than necessary.

ashutosh ahirwal’s picture

ashutosh ahirwal commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update

Issue summary updated Please review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.x-1.x-dev » 2.0.x-dev
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs reroll

The warnings/errors should be fixed in the 2.0.x branch.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

bharath-kondeti made their first commit to this issue’s fork.

bharath-kondeti’s picture

bharath-kondeti commented
Assigned: Unassigned » bharath-kondeti

There are only issues in 8 files against 2.0.x as mentioned below:

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\modules\single_page_site_next_page\src\EventSubscriber\AlterSinglePageSiteOutput.php
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
16 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\single_page_site.install
----------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\single_page_site.module
---------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\src\Controller\SinglePageSiteController.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
20 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
96 | WARNING | [ ] Unused variable $menu.
178 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\src\Event\EventSinglePageSiteAlterOutput.php
------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
11 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\src\Event\SinglePageSiteEvents.php
--------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
9 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
--------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\src\Form\SinglePageSiteConfigForm.php
----------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AND 31 WARNINGS AFFECTING 34 LINES
----------------------------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
15 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
71 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
76 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
81 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
85 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
89 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
98 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
107 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
111 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
115 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
119 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
122 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
123 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
126 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
128 | ERROR | [x] Use null coalesce operator instead of ternary operator.
133 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
138 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
142 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
146 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
150 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
155 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
161 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
162 | ERROR | [x] Use null coalesce operator instead of ternary operator.
167 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
172 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
175 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
179 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
182 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
198 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
205 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
211 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
241 | WARNING | [ ] t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
251 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------------------------------------------

FILE: C:\Practice\xampp\htdocs\locald9site\web\modules\contrib\single_page_site-3150482\src\Manager\SinglePageSiteManager.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
27 | WARNING | [ ] The class short comment should describe what the class does and not simply repeat the class name
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

Time: 644ms; Memory: 4MB

bharath-kondeti’s picture

bharath-kondeti commented
Assigned: bharath-kondeti » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3150482-16.patch13.71 KB

Updated the patch with changes. Please review

anjali mehta’s picture

anjali mehta commented
StatusFileSize
new Before-patch.png145.24 KB
new After-patch.png118.71 KB

Hello @bharath-kondeti
Tested patch on Drupal 9.5.9 and php version 8.1. Patch applied cleanly. But getting error on running the phpcs. Adding screenshot for Your references. Please have a look.
Thank You for your support.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
Issue tags: -Needs reroll 
- * Class AlterSinglePageSiteOutput.
+ * Class to providing functionality for AlterSinglePageSiteOutput.

No, a class does not provide functionality for itself. That does not explain which functionality it provides, though.

+  /**
+   * The Entity Query Factory service.
+   *
+   * @var \Drupal\Core\Entity\Query\QueryFactoryInterface
+   */
+  protected $entityQueryFactory;

The entity query service. is sufficient.

    *   Config factory to load/save config.
    * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
    *   Core module handler.
+   * @param \Drupal\Core\Entity\Query\QueryFactoryInterface $entityQueryFactory
+   *   The Entity Query Factory.
    */

Since that documentation comment is changed, Core module handler. should be replaced with The module handler.

anjali mehta’s picture

anjali mehta commented
Status: Needs work » Needs review
StatusFileSize
new fixed.patch2.13 KB

Updated the patch with changes.

Please review.
Thank You.

anjali mehta’s picture

anjali mehta commented
StatusFileSize
new update.patch14.84 KB

Updated the patch with changes.

Please review.
Thank You.

anjali mehta’s picture

anjali mehta commented
roberttabigue’s picture

roberttabigue commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new phpcsbeforepatch.png774.4 KB
new phpcsafterpatch.png74.79 KB

Hi,

I reviewed the changes and confirmed the Patch was applied cleanly to the Single Page Site module against 2.0.x-dev on Drupal 9.5.10.

Checking patch modules/single_page_site_next_page/src/EventSubscriber/AlterSinglePageSiteOutput.php...
Checking patch single_page_site.module...
Checking patch src/Controller/SinglePageSiteController.php...
Checking patch src/Event/EventSinglePageSiteAlterOutput.php...
Checking patch src/Event/SinglePageSiteEvents.php...
Checking patch src/Form/SinglePageSiteConfigForm.php...
Checking patch src/Manager/SinglePageSiteManager.php...
Applied patch modules/single_page_site_next_page/src/EventSubscriber/AlterSinglePageSiteOutput.php cleanly.
Applied patch single_page_site.module cleanly.
Applied patch src/Controller/SinglePageSiteController.php cleanly.
Applied patch src/Event/EventSinglePageSiteAlterOutput.php cleanly.
Applied patch src/Event/SinglePageSiteEvents.php cleanly.
Applied patch src/Form/SinglePageSiteConfigForm.php cleanly.
Applied patch src/Manager/SinglePageSiteManager.php cleanly.

And all PHPCS errors have been fixed.

I ran this command:
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml single_page_site/

Please see the attached files for reference.

I'm moving this now to RTBC.

Thank you!

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Reviewed & tested by the community » Needs work

The patch could apply, but the changes are not correct.

 /**
- * Class SinglePageSiteController.
+ * Class to providing functionality for SinglePageSiteController.
  *
  * @package Drupal\single_page_site\Controller
  */
@@ -93,7 +93,7 @@ class SinglePageSiteController extends ControllerBase {

The description is still saying the class provides functionality for itself, which is not correct, and it does not explain which functionality the class provides.

-    if ($menu = $this->manager->getMenu()) {
+    if ($this->manager->getMenu()) {

Given the method name, that change does not seem correct. If the $menu variable is not used from render(), there is a bug that needs to be fixed.

 /**
- * Class EventSinglePageSiteAlterOutput.
+ * Class to providing functionality for EventSinglePageSiteAlterOutput.
  *
  * @package Drupal\single_page_site\Event
  */
 /**
- * Class SinglePageSiteEvents.
+ * Class to providing functionality for SinglePageSiteEvents.
  *
  * @package Drupal\single_page_site\Event
  */
 /**
- * Class SinglePageSiteConfigForm.
+ * Class to providing functionality for SinglePageSiteConfigForm.
  *
  * @package Drupal\single_page_site\Form
  */

What I reported for a previous change holds true in these cases too.

+  /**
+   * The entity query service.
+   *
+   * @var \Drupal\Core\Entity\Query\QueryFactoryInterface
+   */
+  protected $entityQueryFactory;

The entity query factory. is better, since the entity query service is a different thing.

+   * @param \Drupal\Core\Entity\Query\QueryFactoryInterface $entityQueryFactory
+   *   The Entity Query Factory.

Only the first word must be capitalized.

bharath-kondeti opened merge request !4

zkhan.aamir’s picture

zkhan.aamir commented
Status: Needs work » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes

apaderno opened merge request !5

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

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

shank115’s picture

shank115 commented

I ran this command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml single_page_site/ and found no issues or warnings.

apaderno changed the visibility of the branch 3150482-fix-the-issues to hidden.

roberttabigue’s picture

roberttabigue commented
Status: Needs review » Needs work
StatusFileSize
new Enabled_Gitlab_CI.png554.4 KB

Hi @avpaderno,

The Gitlab CI failed to run the Jobs: https://git.drupalcode.org/issue/single_page_site-3150482/-/jobs/1066014.

Please see the attached file for reference.

I'm moving this to 'Needs work' for now.

Thank you.

f0ns’s picture

f0ns commented
Status: Needs work » Fixed

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.