- 🇮🇳India arpitk
The patch #4 applied cleanly however there is still following errors/warnings reported :
FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/EventSubscriber/OrderEventSubscriber.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------
52 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
166 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------------------FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/Form/StockInventoryControlForm.php
-------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------------------------------------------
142 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
145 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
164 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------------------FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/src/OrderProcessor/AvailabilityOrderProcessor.php
------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------------
30 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
------------------------------------------------------------------------------------------------------------------------------FILE: /home/arpitkayare/drupal_9.5/web/modules/contrib/commerce_simple_stock/js/inventory_control.js
----------------------------------------------------------------------------------------------------
FOUND 44 ERRORS AFFECTING 22 LINES
----------------------------------------------------------------------------------------------------
3 | ERROR | [x] Space before opening parenthesis of function call prohibited
4 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
4 | ERROR | [x] Expected 1 space before opening brace; found 0
11 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
11 | ERROR | [x] Expected 1 space before opening brace; found 0
13 | ERROR | [x] Functions must not contain multiple empty lines in a row; found 2 empty lines
15 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
17 | ERROR | [x] Expected 1 space before "=="; 0 found
17 | ERROR | [x] Expected 1 space after "=="; 0 found
23 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
23 | ERROR | [x] Expected 1 space before opening brace; found 0
29 | ERROR | [x] Whitespace found at end of line
30 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
30 | ERROR | [x] Expected 1 space before opening brace; found 0
37 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
42 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
53 | ERROR | [x] Whitespace found at end of line
66 | ERROR | [x] Expected 1 space before "+"; 0 found
66 | ERROR | [x] Expected 1 space after "+"; 0 found
71 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
79 | ERROR | [x] Expected 1 space before "+"; 0 found
79 | ERROR | [x] Expected 1 space after "+"; 0 found
79 | ERROR | [x] Expected 1 space before "+"; 0 found
79 | ERROR | [x] Expected 1 space after "+"; 0 found
80 | ERROR | [x] Expected 1 space before "+"; 0 found
80 | ERROR | [x] Expected 1 space after "+"; 0 found
80 | ERROR | [x] Expected 1 space before "+"; 0 found
80 | ERROR | [x] Expected 1 space after "+"; 0 found
80 | ERROR | [x] Expected 1 space before "+"; 0 found
80 | ERROR | [x] Expected 1 space after "+"; 0 found
81 | ERROR | [x] Expected 1 space before "+"; 0 found
81 | ERROR | [x] Expected 1 space after "+"; 0 found
81 | ERROR | [x] Expected 1 space before "+"; 0 found
81 | ERROR | [x] Expected 1 space after "+"; 0 found
82 | ERROR | [x] Expected 1 space before "+"; 0 found
82 | ERROR | [x] Expected 1 space after "+"; 0 found
82 | ERROR | [x] Expected 1 space before "+"; 0 found
82 | ERROR | [x] Expected 1 space after "+"; 0 found
82 | ERROR | [x] Expected 1 space before "+"; 0 found
82 | ERROR | [x] Expected 1 space after "+"; 0 found
89 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
90 | ERROR | [x] Inline control structures are not allowed
98 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
99 | ERROR | [x] Equals sign not aligned correctly; expected 1 space but found 2 spaces
----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 44 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------command used phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,js .
Working on updating the patch.
Thanks!
- Issue was unassigned.
- Status changed to Needs review
about 1 year ago 2:08pm 7 June 2023 - Status changed to RTBC
about 1 year ago 1:39pm 9 June 2023 - 🇵ðŸ‡Philippines clarkssquared
Hi arpitk
I applied your patch #8 to my local with Drupal 9.5.9 and confirmed that your patch fixed all the PHPCS errors/issue,
Please look at the screenshots I provided for your reference.
Thank you.
- Status changed to Needs work
about 1 year ago 9:18am 10 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
The patch could fix the reported errors/warnings, but it contains wrong changes.
/** - * Class CommerceSimpleStockController. + * Implementation of Commerce Simple Stock Controller. */
There is no need to start a class description with Implementation of, especially when it is followed by the class name (which is essentially saying the class is an implementation of itself). That description does not say what the class does.
+ /** + * The entity_type.manager service. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface + * The entity_type.manager service. + */ + protected $entityTypeManager;
The property description is only given before the
@var
line. The entity type manager. is a sufficient description.+ /** + * The config factory service. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; +
There is no need to say it is a service.
- $events['commerce_order.place.post_transition'] = ['postTransitionOrder', -100]; - $events['commerce_order.cancel.post_transition'] = ['postTransitionCancelOrder', -100]; + $events['commerce_order.place.post_transition'] = ['postTransitionOrder', + -100, + ]; + $events['commerce_order.cancel.post_transition'] = ['postTransitionCancelOrder', + -100, + ];
- if ($order && !in_array($order->getState()->value, ['draft', 'canceled'])) { + if ($order && !in_array($order->getState()->value, ['draft', + 'canceled', + ])) { $purchasedEntity = $item->getPurchasedEntity();
Code lines are not required to be shorter than 81 characters. Line length and wrapping → , part of the Drupal coding standards, says:
- Lines containing longer function names, function/class definitions, variable declarations, etc are allowed to exceed 80 characters.
- Control structure conditions may exceed 80 characters, if they are simple to read and understand.
(Emphasis is mine.)
In both the cases, the existing code is simpler to read and understand.
+ /** + * Constructs a new StockInventoryControlForm object. + * + * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager + * The entity type manager. + */
The description for a constructor must start with
Constructs a new
followed by the class name (including its namespace), and end withobject.
- Status changed to Needs review
about 1 year ago 9:25am 12 June 2023 - Status changed to Needs work
about 1 year ago 11:30am 12 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- if(e.keyCode==13 && ($(this).val() != '')) { + if(e.keyCode == 13 && ($(this).val() != '')) {
There must be a space between if and the opening parenthesis.
+ * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory service.
There is no need to say it is a service.
/** - * Class StockInventoryConfigForm. + * Implementing Stock Inventory Config Form. */
That description is still wrong for the reasons I reported in my previous comment.
+ /** + * The entity_type.manager service. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface + * The entity_type.manager service. + */ + protected $entityTypeManager;
See my previous comment for what needs to be changed.
+ /** + * Constructs a new StockInventoryControlForm object. + *
That has not been changed since my previous comment.
+ /** + * The config factory service. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory;
It is sufficient to say The config factory.
+ /** + * Constructs a new AvailabilityOrderProcessor object. + * + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory service. + */
That needs to be changes as per my previous comment, which holds true for every constructor description where the class namespace is missing.
Furthermore, the issue summary should always describe what the issue is trying to fix and, in the case of coding standards issues, show which command and arguments have been used and which report that command shown. In this way, project maintainers can verify the patch/MR fixes all the warnings/errors.
- Status changed to Needs review
about 1 year ago 12:33pm 12 June 2023 - 🇮🇳India sakthi_dev
Created a patch based on comment #12. Please review.
- 🇮🇳India Preethy_ray
Hi,
Above patch addressed the #12 comments, but there are still few errors and warnings found that need correction. - 🇮🇳India dev16.addweb
Hi here is the updated patch addressing #14. Please review.
Thanks!