- Issue created by @urvashi_vora
- 🇵🇭Philippines kenyoOwen
Hi urvashi_vora
I applied the patch coding-standard-fixes.patch → to the “Scaleflex Cloudimage” module against Version 1.0.x-dev and there is still remaining issue. I will transition this to needs work so others can fix the remaining issue. Please see the screenshots attached.
For your review.
Thank you. - Status changed to Needs review
about 1 year ago 1:39pm 20 April 2023 - Status changed to RTBC
about 1 year ago 2:52pm 20 April 2023 - 🇵🇭Philippines kenyoOwen
Hi sakthi_dev
I applied the patch #3 to the “Scaleflex Cloudimage” module against Version 1.0.x-dev and confirmed that it fixed the remaining issue. Please see the screenshots attached.
For your review.
Thank you. - Status changed to Needs work
about 1 year ago 8:36pm 1 May 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
+/** + * @file + * Module File. + */
The usual short description is Hook implementations for the [module name] module.
+/** + * Cloud Image Service. + */ class CloudimageService {
An article is missing from the short description.
Image and Service are misspelled, since they are not at the beginning of the sentence.+ /** + * Router for get the base url. + * + * @var \Drupal\Core\Routing\RequestContext
url is misspelled.
That class is not a router.- $message = "{ \n" . "status".":"."success".",\n"."count_added".":". count($images) ."\n }"; + $message = "{ \n" . "status" . ":" . "success" . ",\n" . "count_added" . ":" . count($images) . "\n }"; $this->logger->notice($message);
Strings passed as first argument to logger methods must use placeholders, not be a concatenation of strings.
/** - * Class CloudimageForm. + * Cloud image Form. * * @package Drupal\scaleflex_cloudimage\Form */ class CloudimageForm extends ConfigFormBase {
The short description must not contain the class name.
Form is misspelled.+ /** + * {@inheritdoc} + */ private $cloudimageConfig;
{@inheritdoc}
is not used for properties that are not inherited from the parent class or defined from an interface.+ /** + * {@inheritdoc} + */ public function __construct(ConfigFactoryInterface $config_factory) {
It is not even used for class constructors.
- Status changed to Needs review
7 months ago 7:37am 13 December 2023 - Status changed to Needs work
7 months ago 7:43am 13 December 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
+/** + * @file + * Adds CloudImage source to image variables if the route is not node edit form. + */
The usual description for a .module file is Hook implementations for the [module name] module. where [module name] is the name of the module given in its .info.yml file.
/** * Implements hook_page_attachments(). */ + +/** + * Adds CloudImage-related attachments to the page. + */ function scaleflex_cloudimage_page_attachments(array &$attachments) {
Each function/method must have only a single documentation comment. The existing one is already correct.
-function scaleflex_cloudimage_preprocess_html(array &$variables){ + +/** + * Adds CloudImage attributes to HTML variables for specific pages. + */ +function scaleflex_cloudimage_preprocess_html(array &$variables) {
That is not the correct documentation comment for a preprocess hook. It should be similar to Implements hook_preprocess_HOOK() for image.html.twig.
+/** + * Provides methods to interact with the Cloudimage service. + * + * This class encapsulates functionality related to Cloudimage services, + * allowing interaction with Cloudimage APIs and related operations. + */
The long description is not necessary.
+ /**
+ * Constructs a new CloudimageService object.
+ *
+ * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+ * The configuration factory service.
+ * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger
+ * The logger channel factory service.
+ * @param \Drupal\Core\File\FileSystemInterface $file_system
+ * The file system service.
+ * @param \GuzzleHttp\ClientInterface $http_client
+ * The Guzzle HTTP client.
+ * @param \Drupal\Core\Routing\RouterInterface $router
+ * The router service.
+ */The documentation comment for constructors is not mandatory anymore, If it is given, the description must be Constructs a new [class name] object. where [class name] includes the class namespace.
- $response = $client->post('https://warmup.api.cloudimage.com/warmup/urls', [ + // $response = new Response(); + // $client = new Client(); + $response = $this->httpClient->post('https://warmup.api.cloudimage.com/warmup/urls', [ 'headers' => [ 'X-Client-Key' => $config->get('client_key'),
Code that is not necessary is removed, not commented out.
+ /** + * Represents the Cloudimage configuration. + * + * @var \Drupal\Core\Config\ImmutableConfig + */ private $cloudimageConfig;
The config object. is sufficient.
- Status changed to Needs review
7 months ago 8:29am 13 December 2023 - Status changed to Needs work
7 months ago 12:11pm 13 December 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
+/** + * @file + * Hook implementations for the Scaleflex Cloudimage. + */
Hook implementations for the Scaleflex Cloudimage module.
/** * Implements hook_page_attachments(). + * + * Adds CloudImage-related attachments to the page. */ function scaleflex_cloudimage_page_attachments(array &$attachments) {
The longer description essentially says what the short description already says; it is not necessary.
+/** + * Implements hook_preprocess_html(). + */ +function scaleflex_cloudimage_preprocess_html(array &$variables) {
The short description must be Implements hook_preprocess_HOOK() for [template filename].
/** * Implements hook_cron(). */ + +/** + * Perform CloudImage-related tasks during cron runs. + */ function scaleflex_cloudimage_cron() {
Each function/method must have a single documentation comment. This patch is adding a new documentation comment to a function which has already a documentation comment.
+ /** + * The Cloudimage settings. + * + * @var \Drupal\Core\Config\ImmutableConfig + */ + protected $cloudimageSettings;
The config object. is sufficient.
+ /** + * Constructs a new class instance. + * + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The configuration factory service. + */
The documentation comment for constructors is not mandatory anymore, If it is given, the description must be Constructs a new [class name] object. where [class name] includes the class namespace.
That holds true for every class constructors. - Assigned to nitin_lama
- Status changed to Needs review
7 months ago 12:03pm 14 December 2023 - Issue was unassigned.
Hi, Reviewed the patch #12, applied cleanly and fixes all errors. And addresses the comments #10.
Thank you!