Fix the issues reported by phpcs

Working on it.

I'm having the same problem, the patch doesn't apply, what should i do? If anyone can help me...

Skipped patch 'README.txt'.
Skipped patch 'src/NgLightbox.php'.
Skipped patch 'src/Tests/NgLightboxTest.php'.
Checking patch src/Form/NgLightboxSettingsForm.php...
error: while searching for:
namespace Drupal\ng_lightbox\Form;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
use Drupal\Core\Form\ConfigFormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\ng_lightbox\NgLightbox;
use Symfony\Component\DependencyInjection\ContainerInterface;

class NgLightboxSettingsForm extends ConfigFormBase {

  /**
   * @var \Drupal\Core\Config\ConfigFactoryInterface
   */
  protected $config;

error: patch failed: src/Form/NgLightboxSettingsForm.php:3
error: src/Form/NgLightboxSettingsForm.php: patch does not apply

I tried to reroll and merge the las two patches as they had different changes.

The test are failing! I could not fix it, so keeping in needs work.

Hi I did a reroll, please revise.

I will review this.

I reviewed patch #17 its working but still I can see some errors and warnings.

FILE: ...:\xampp\htdocs\abc\drupal\modules\ng_lightbox\ng_lightbox.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\drupal\modules\ng_lightbox\src\Form\NgLightboxSettingsForm.php
----------------------------------------------------------------------
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: ...:\xampp\htdocs\abc\drupal\modules\ng_lightbox\src\NgLightbox.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"
112 | WARNING | [ ] \Drupal calls should be avoided in classes, use
| | dependency injection instead
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...mpp\htdocs\abc\drupal\modules\ng_lightbox\src\NgLightboxPass.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"
18 | WARNING | [ ] Unused variable $id.
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...abc\drupal\modules\ng_lightbox\src\NgLightboxServiceProvider.php
----------------------------------------------------------------------
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: ...l\modules\ng_lightbox\tests\src\Functional\NgLightboxWebTest.php
----------------------------------------------------------------------
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\drupal\modules\ng_lightbox\tests\src\Kernel\NgLightboxTest.php
----------------------------------------------------------------------
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: ...abc\drupal\modules\ng_lightbox\tests\src\Unit\NgLightboxTest.php
----------------------------------------------------------------------
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
----------------------------------------------------------------------

SO moving to needs work.

Some of the errors and warnings are fixed now.
Created MR !7 for this issue.
Please review.

Thank you.

@charchil thank you for opening a MR!, that helps on reviews a lot.
please be careful with naming branches for a MR, the way it is now the branch 2.x already exists (because is the branch for this version) so git checkout -b '2.x' --track ng_lightbox-3152816/'2.x' cannot be applied.
We need to use something like git checkout -b '3152816' --track ng_lightbox-3152816/'2.x' instead.

Also, please don't break the line of a code in the README.md.
That line is meant to help people just copy and past the code on their terminals, but with the line break, it will also break in the terminal and therefore, not work as expected.

There is still one error to be fixed

FILE: ...\modules\ng_lightbox\src\NgLightbox.php
----------------------------------------------------------------------
FOUND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
112 | WARNING | [ ] \Drupal calls should be avoided in classes, use
| | dependency injection instead
----------------------------------------------------------------------

I will try to fix it.

Hi I fixed the tests please revise.

The last submitted patch, 16: 3152816-16.patch, failed testing. View results →

Te tests are passing and the DI was fixed. \o/

The remaining cs of

FILE: /.../modules/contrib/ng_lightbox/README.md
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
 17 | WARNING | Line exceeds 80 characters; contains 87 characters
----------------------------------------------------------------------------------------------

refers to
compass compile --no-sourcemap --no-debug-info --force -e production sass/lightbox.scss
that I consider that should stay this way,

I just added Escaping Backticks to maintain as a code in the visualization and be shorter.
=)

I will review it.

Hi coding standards has been fixed.

Module keeps working as expected

Test arent't breaking.

Moving to RTBC.

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.

Hi I just fixed the characters limit. Please revise.

Hi we can move to RTBC I ran Drupal phpcs command and it not exist anymore

pcbf --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml .

No fixable errors were found

Time: 152ms; Memory: 8MB

Module keeps working as expected.

PHPCS were fixed.

Moving to RTBC.

The person who provided the last patch or done the last commit in the MR cannot change the status to Reviewed & tested by the community.

+   * @param Symfony\Component\HttpFoundation\RequestStack $request
+   *   The request method.

That is not a method.

-    // @TODO, decide whether we want to try and support paths or to adopt routes
+    // @todo , decide whether we want to try and support paths or to adopt routes

What follows @todo is a sentence: It starts with a capitalized word and ends with a period.
The comma between @todo and the sentence is not used, either when there are spaces between @todo and the comma or not.

Also, if I run phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig for the 2.x branch of the module, I get a slightly different report.

HI I will see it.

+   * @param Symfony\Component\HttpFoundation\RequestStack $request
+   *   The request method.

That is not a method.

runing phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig returns a clean terminal.

at src/NgLightbox.php line 111

-     // @TODO, decide whether we want to try and support paths or to adopt routes
+    // @todo Decide whether we want to try and support paths or to adopt routes.

but the full sentence is broke in two, the full sentence is

// Decide whether we want to try and support paths or to adopt routes
// like core is trying to force us into.

so it should have a captalized word but leave the period only on the second line.

everything else looks good to me,

Thank you for the quick response.

Everything looks good to me, moving to RTBC

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

Fixed as requested.

all phpcs clean.