- Issue created by @dineshkumarbollu
- Issue was unassigned.
- Status changed to Needs review
about 1 year ago 6:34am 21 April 2023 - 🇮🇳India Raveen Kumar
I have reviewed patch #2. And it seems as per coding standards. Thank you.
- Status changed to Needs work
about 1 year ago 4:48pm 21 April 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
I am reviewing the patch in comment #2.
+ * @param string $plugin_id + * The plugin_id parameter. + * @param mixed $plugin_definition + * The Plugin_definition parameter.
It is not how parameters are described. For example, there is no need to say that a parameter is... a parameter.
+ * @param \Drupal\Core\Field\FieldDefinitionInterface $field_definition + * The FeildDefintion service.
There are two typos.
- Status changed to Needs review
about 1 year ago 6:03pm 21 April 2023 - 🇮🇳India Soham Sengupta
Hi, I have updated the parameter comments.
- 🇮🇳India chanderbhushan
@apaderno, could you please check in Drupal core code there also same defined
- Status changed to Needs work
about 1 year ago 7:39am 22 April 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- * @var EntityTypeManagerInterface + * Entity Type Manager service. + * + * @var \Drupal\Core\Entity\EntityTypeManagerInterface */
In English, there are rules about which words are written capitalized. On Entity Type Manager only the first word is correctly spelled.
There is a missing article, in the short description.+ * @param string $plugin_id + * The plugin_id for the formatter. + * @param mixed $plugin_definition + * The plugin implementation definition.
It is redundant to say that the
$plugin_id
parameter is the plugin_id of the formatter, since that is repeating the parameter name.
The correct description for those parameters can be seen in constructors likePluginBase::__construct()
, for example.+ * @param string $label + * The formatter label display setting.
It is sufficient to say The formatter label.
* @param array $third_party_settings - * @param EntityTypeManagerInterface $entity_type_manager + * Any third party settings. + * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager + * The Entity type manager service.
Any>/em> is not necessary.
Entity is misspelled, since it is not at the beginning of the sentence (contrary to The). - Status changed to Needs review
about 1 year ago 5:41pm 24 April 2023