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
Comments
Comment #3
prasaddeole commentedComment #4
prasaddeole commentedComment #5
bhanu951 commentedTest Bot is Green and Changes look good.
+1 From me for RTBC.
Comment #6
lamp5Comment #7
kunalgautam commentedPatch #3 is failed to apply.
Updated patch accordingly.
Comment #8
kunalgautam commentedComment #9
akram khanchecked patch #7 it apply cleanly and Fix phpcs coding standards issue
Comment #10
akram khanForgot to added SS
Comment #11
avpadernoThe 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.
Comment #12
avpadernourl is not spelled correctly.
Comment #14
kunalgautam commented@apaderno thanks for reviewing. I have created MR of patch #7 and include commented change as well.
Please review.
Comment #15
kunalgautam commentedComment #16
avpadernoThat comment is still repeating the class name.
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.Comment #18
rassoni commentedComment #19
avpadernoComment #20
avpadernoThe last line must be indented.
That is still repeating the class name. Splitting the class name in two words does not change that is the class name.
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.
Comment #24
nitin_lamaComment #25
nitin_lamaRebased and pushed changes. Updating IS for the remaining issues.
Comment #26
nitin_lamaComment #28
lamp5