- 🇮🇹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 new
followed 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 1 year ago 4:46pm 15 May 2023 - Status changed to Needs work
over 1 year 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 1 year ago 12:31pm 16 May 2023 - Status changed to Needs work
over 1 year 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 1 year 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 1 year 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 1 year ago 9:57am 29 May 2023 - Status changed to Needs work
over 1 year 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 1 year 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
5 months ago 4:41pm 15 August 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
The changes introduced from the patch are correct.
- Assigned to f0ns