- 🇮🇹Italy apaderno Brescia, 🇮🇹
/** - * Class StickyManager + * The class StickyManager. *The description must describe what the class does, not repeat the class name.
+ /** + * {@inheritdoc} + */ protected $config;cannot be used in a class without parent classes nor that does not implement interfaces.
+ /** + * Construct sticky configuration. + */ public function __construct(ConfigFactoryInterface $config) {The description for a constructor must start with
Constructs a newfollowed by the class name (including its namespace), and end withobject.It must also describe the parameters and the return value, if the method has any of them.* @return array + * Return settings of JS. */ public function getJsSettings() {The return value description must not start with Return nor Returns.
- Assigned to arpitk
- Issue was unassigned.
- Status changed to Needs review
over 2 years ago 4:46pm 15 May 2023 - Status changed to Needs work
over 2 years ago 9:05pm 15 May 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
/** - * Class StickyManager + * The class provides Js settings.I am not sure that description is accurate.
+ /** + * The config factory service. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ protected $config;The config factory. is sufficient.
+ /** + * Constructs a new StickyManager object. + *The class name is missing its namespace.
* @return array + * The array with Settings of JS. */ public function getJsSettings() {The return value description repeats what the method description already says.
with Settings should be both spelled with lowercase letters. - Status changed to Needs review
over 2 years ago 12:31pm 16 May 2023 - Status changed to Needs work
over 2 years ago 3:01pm 17 May 2023 - 🇵ðŸ‡Philippines roberttabigue
Hi,
I'm still seeing PHPCS error after applying Patch #11 to the Sticky module with version 2.0.x-dev and with Drupal core version 9.5.8.
Moving this to Needs work for now.
Kindly refer to the attached screenshots, please. - Status changed to Needs review
over 2 years ago 9:28am 26 May 2023 - 🇮🇳India mrinalini9 New Delhi
Updated patch #11 by addressing #12, please review it.
Thanks!
- Status changed to Needs work
over 2 years ago 1:24pm 26 May 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
/** - * Class StickyManager + * The Class for returning array of Js Settings. *I am not still sure that is a correct description.
There are misspelled words that needs to be corrected: The only capitalized word must be the first.
It is also not necessary to add The Class for in a class description.+ * * @return array + * The array with Settings of JS.Settings is misspelled, since it is not the first word in the description, a proper noun, nor an acronym.
Instead of Settings of JS, it should be JavaScript settings. - Assigned to nitin_lama
- Issue was unassigned.
- Status changed to Needs review
over 2 years ago 9:57am 29 May 2023 - Status changed to Needs work
over 2 years ago 6:12pm 29 May 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
-2. Download the Sticky library (version 1.0.4: https://github.com/garand/sticky), -rename the folder to sticky and place it in /libraries/ -in your drupal root. +2. Download the Sticky library +(version 1.0.4: https://github.com/garand/sticky), +rename the folder to sticky and place it in /libraries/ in your drupal root.Since that text is part of a list, the second and the third line need to be indented.
The second line is 50 characters; the text can be reformatted to have lines that reach 80 characters.
drupal is misspelled; as proper noun, it should be capitalized./** - * Class StickyManager + * Provides a method to return array of js settings.Assuming that description explains the class purpose, there must an undefined article before array and js should be replaced by the full language name.
- Assigned to nitin_lama
- Issue was unassigned.
- Status changed to Needs review
over 2 years ago 9:44am 30 May 2023 Hi, Reviewed the patch at #21, applied cleanly and fixes all the phpcs errors/warnings and also addresses the comments mentioned at #19.
Thank you!- Status changed to RTBC
about 1 year ago 4:41pm 15 August 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
The changes introduced from the patch are correct.
- Assigned to f0ns