- Issue created by @dineshkumarbollu
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 5:05am 13 April 2023 - ๐ฎ๐ณIndia saket-001
thanks @dineshkumarbollu for the patch. Applied successfully and fixes all PHPCS error.
sharing the SS. - Status changed to RTBC
over 1 year ago 6:00am 13 April 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
The patch could apply, but it is not correct.
- * @param $plugin_id - * @param $plugin_definition + * The array of values. + * @param int $plugin_id + * The Plugin_id value. + * @param string $plugin_definition + * Definition of Plugin.
array of values is too generic for that parameter.
Plugin_id is not an English word.
The last line is missing an article and misspelling Plugin, since that word is not at the sentence beginning.* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager + * EntityTypeMangager service.
There is still a missing article.
The service name is also misspelled.
The entity type manager. is a better description; there is no need to say that a manager is a service, since in Drupal that is normal.* @return string + * Return the value of type string
That description does not make sense. Furthermore, saying that the returned value is a string (which I guess is what that comment is supposed to say) is quite vague and useless, since that is information already given from the parameter type-hinting. The correct description should say what exactly the returned value is, not its type.
- * @param $field + * @param string $field + * Define field value.
That is a too generic description.
* @return mixed + * Reurns the value of Mixed
There is a typo.
value of Mixed does not make sense and it does not say what the method returns.+ * @param array $values + * The array of values. + * @param string $field + * Define field value. + * @param string $relationship + * Define relationship value.
Those descriptions are not helpful at all.
It is not that, for a$random_value
parameter, the description can be Define random_value value. because that is wrong and too generic.+/** + * @file + * Define the Hooks of the module. + */ +
The usual comment is Hook implementations for the [module name] module.
Hooks is misspelled, since it is not at the beginning of the sentence. - Status changed to Needs work
over 1 year ago 12:39pm 29 April 2023 - First commit to issue fork.
- Assigned to shalini_jha
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 4:25pm 29 April 2023 - ๐ฎ๐ณIndia shalini_jha
Added the patch for fixes of listed issues. please review.
- ๐ฎ๐ณIndia djsagar
Hi @all,
After applied patch #10, all phpcs issue are fixed.
RTBC ++
- Status changed to RTBC
over 1 year ago 11:03am 25 August 2023 - ๐ง๐ทBrazil elber Brazil
Hi I also reviewed it.
Patch applies cleanly.
I ran
phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml
All the phpcs errors has been removed.
Module keeps good.
Moving to RTBC.
- Status changed to Needs work
over 1 year ago 12:18pm 25 August 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
* @param array $configuration - * @param $plugin_id - * @param $plugin_definition + * An array of configuration used by the plugin. + * @param int $plugin_id + * The plugin_id for the handler. + * @param string $plugin_definition + * The plugin implementation definition.
The type and description of the first three parameters need to be:
- array
$configuration:
A configuration array containing information about the plugin instance. - string
$plugin_id
: The plugin ID. - mixed
$plugin_definition
: The plugin implementation definition.
* Determine if the field comes from a relationship. * - * @param $field + * @param string $field + * The name of the field for which to retrieve the relationship.
/** * Determine if the field is rewritten/altered. * - * @param $field + * @param string $field + * The name of the field to retrieve the rewrite status. * * @return mixed + * The rewrite status for the provided field,which can be mixed value. */
Since those documentation comments are changed, Determine must be declined on the third person singular.
- * @param $values - * @param $field - * @param $relationship + * Defining the values. + * + * @param array $values + * An array of values used by the plugin. + * @param string $field + * The name of the field to retrieve the relationship entity. + * @param string $relationship + * The name of the relationship to retrieve. * * @return \Drupal\Core\Entity\EntityInterface|null + * An entity representing the relationship, or null if not found. + * * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException * @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException */ - protected function getRelationshipEntity($values, $field, $relationship) { + protected function getRelationshipEntity(array $values, $field, $relationship) {
Defining the values. is not the correct description for a
getRelationshipEntity()
method.+/** + * @file + * Hook implementations for the views_cumulative_field module.
views_cumulative_field must be replaced from the module name, which is different from the module machine name.
+ // Allow users to specify how precise the results should be. + // Useful for divisions.
Allow must be declined on the third person singular.
The other sentence is missing verb and subject (It is). - array
- First commit to issue fork.
- ๐ฎ๐ณIndia Yashaswi18
Addressed above comments in this patch. Please review.
- Status changed to Needs review
11 months ago 2:20pm 1 February 2024 - ๐ฎ๐ณIndia Shank115
I applied patch provided in #16, applies cleanly. Ran phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig views_cumulative_field/, found no phpcs errors.
Checking patch src/Plugin/views/field/CumulativeField.php... Checking patch views_cumulative_field.views.inc... Applied patch src/Plugin/views/field/CumulativeField.php cleanly. Applied patch views_cumulative_field.views.inc cleanly.
- Status changed to Needs work
10 months ago 2:40pm 6 March 2024 - ๐บ๐ธUnited States andileco
Would someone please make a merge request from the latest patch? The code looks good, but I like to see the pipeline results that run on merge requests.
- First commit to issue fork.
- Status changed to Needs review
9 months ago 8:57am 11 March 2024 -
andileco โ
committed ef58b067 on 2.0.x authored by
pray_12 โ
Issue #3353889 by Yashaswi18, shalini_jha, dineshkumarbollu, pray_12,...
-
andileco โ
committed ef58b067 on 2.0.x authored by
pray_12 โ
- Status changed to Fixed
9 months ago 2:21pm 11 March 2024 Automatically closed - issue fixed for 2 weeks with no activity.