Problem/Motivation

FILE: /home/system/Documents/contribution/custom_breadcrumbs-3318181/src/BreadcrumbBuilder.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 151 | WARNING | The removal-version 'custom_breadcrumbs' does not match the lower-case machine-name standard: drupal:n.n.n or project:n.x-n.n or project:n.x-n.n-label[n] or project:n.n.n or
     |         | project:n.n.n-label[n]
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 126ms; Memory: 10MB

FILE: /home/system/Documents/contribution/custom_breadcrumbs-3318181/src/BreadcrumbBuilder.php
----------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
 152 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
----------------------------------------------------------------------------------------------

Time: 96ms; Memory: 8MB

Steps to reproduce

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml custom_breadcrumbs 
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:

Comments

PrasadDeole created an issue. See original summary.

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

prasaddeole’s picture

StatusFileSize
new2.56 KB
prasaddeole’s picture

Assigned: prasaddeole » Unassigned
Status: Active » Needs review
bhanu951’s picture

Status: Needs review » Reviewed & tested by the community

Test Bot is Green and Changes look good.

+1 From me for RTBC.

lamp5’s picture

Status: Reviewed & tested by the community » Needs work
kunalgautam’s picture

StatusFileSize
new4.83 KB
new4.19 KB

Patch #3 is failed to apply.
Updated patch accordingly.

kunalgautam’s picture

Status: Needs work » Needs review
akram khan’s picture

Status: Needs review » Reviewed & tested by the community

checked patch #7 it apply cleanly and Fix phpcs coding standards issue

akram khan’s picture

Forgot to added SS

avpaderno’s picture

Title: Fix phpcs coding standards » Fix the issues reported by phpcs
Status: Reviewed & tested by the community » Needs work
Issue tags: +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

+      '#description' => $this->t('One url per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.',
+        [
+          '@token' => 'https://www.drupal.org/project/token',
+          '@nolink_key' => '<nolink>',
+        ]
+      ),

url is not spelled correctly.

kunalgautam’s picture

@apaderno thanks for reviewing. I have created MR of patch #7 and include commented change as well.
Please review.

kunalgautam’s picture

Status: Needs work » Needs review
avpaderno’s picture

Status: Needs review » Needs work
/**
- * Class BreadcrumbBuilder.
+ * Breadcrumb builder class.

That comment is still repeating the class name.

-      '#description' => $this->t('One url per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.', ['@token' => 'https://www.drupal.org/project/token', '@nolink_key' => '<nolink>']),
+      '#description' => $this->t('One URL per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.',
+        [

URL is an acronym and it is only written in one way. Also, since that sentence is going to be changed, also must start from needs to be corrected, since the correct phrase is must start with.
Then, One url per line, you can use Token module. is a comma-splice sentence. Instead of the comma, it should use a period because those are two different sentences.

+ '#description' => $this->t("A set of patterns separated by a newline. @front_key@ is used to front page. The * character is a wildcard. An example path is /admin/* for every admin pages.",

The correct phrase is is used for the front page, not is used to front page.

+ '#description' => $this->t('One URL per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.',

An article is missing before <a href="@token">Token</a> module.

The Needs work status was also set for the issue summary, which does not show what arguments has been passed to phpcs.

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

rassoni’s picture

Issue summary: View changes
avpaderno’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update
avpaderno’s picture

Status: Needs review » Needs work
- * Configure the user permissions in Administration » People » Permissions:
+ * Configure the user permissions in
+ Administration » People » Permissions:

The last line must be indented.

 /**
- * Class BreadcrumbBuilder.
+ * Breadcrumb builder class.

That is still repeating the class name. Splitting the class name in two words does not change that is the class name.

-      '#description' => $this->t('One url per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.', ['@token' => 'https://www.drupal.org/project/token', '@nolink_key' => '<nolink>']),
+      '#description' => $this->t('One URL per line, you can use <a href="@token">Token</a> module. Url must start from "/". Use @nolink_key if you don\'t want to set a link for the respective title.',
+        [
+          '@token' => 'https://www.drupal.org/project/token',
+          '@nolink_key' => '<nolink>',
+        ]
+      ),

One URL per line, you can use Token module. is a comma-splice sentence (a grammatical error). Instead of a comma, a semicolon or a period must be used.
Url is still a misspelled word. (Acronyms are always in upper-case characters.)
start from is wrong. It is either start with or begin with.

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

nitin_lama changed the visibility of the branch 3318181-fix-phpcs-coding to hidden.

nitin_lama changed the visibility of the branch 3318181-fix-phpcs-coding to active.

nitin_lama’s picture

Assigned: Unassigned » nitin_lama
nitin_lama’s picture

Issue summary: View changes

Rebased and pushed changes. Updating IS for the remaining issues.

nitin_lama’s picture

Assigned: nitin_lama » Unassigned

lamp5’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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