- ๐ฎ๐ณIndia rassoni Bangalore
Rashmisoni โ made their first commit to this issueโs fork.
- Status changed to Needs work
almost 2 years ago 10:48am 21 February 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, report which command has been used, which arguments have been used, and which report that command shown.
- ๐ซ๐ทFrance maro22
Hey,
This module not compatible wth drupal 10 ?!
They are a anthor module like this, for migration site from 7 to 10, please
Thanks - Status changed to Needs review
over 1 year ago 11:19am 3 April 2023 - ๐ฎ๐ณIndia saket-001
patch #5 applied successfully compatible with my version sharing the SS.
- Status changed to Needs work
over 1 year ago 1:24pm 3 April 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
+ /** + * Construct. + */ public function __construct(ConfigFactory $config_factory, VisualizationHandlerManager $handler_manager) {
That is not the short description used for class constructors.
The description for the parameters is missing.+ /** + * Form Id. + */ + public function getFormId() { return 'visualization_settings_form'; }
Since that is a method defined from the parent class, the documentation comment is different.
/** * @file - * Definition of Drupal\visualization\Plugin\VisualizationHandlerManager + * Definition of Drupal\visualization\Plugin\VisualizationHandlerManager. */
@file
is not used for files containing a single class definition.+ /** + * Name. + * + * @var name + */ public $name = 'highcharts'; + /** + * Added Javascript. + * + * @var addedJavascript + */ protected $addedJavascript = FALSE; + + /** + * Available. + * + * @var available + */ protected $available = FALSE;
Short descriptions should not just repeat the property name.
+ * Some default information is: + * - title: the label of the chart + * - type: the internal name of the type of the chart; default types are + * line, pie and column. Specific charting libraries are welcome to + * implement custom chart types as they please. + * - fields: an associative array of internal field names and their labels. + * - xAxis: options concerning the x-axis (if appropriate) + * - yAxis: options concerning the y-axis (if appropriate)
The lines after the first one should be indented.
+ * @return array + * A string that will be the body of the chart div.
If the returned value is a string, it cannot be an array.
-core: 8.x -configure: visualization.settings_form \ No newline at end of file +core_version_requirement: ^8
core_version_requirement
is recognized only starting from Drupal 8.8. Ifcore: 8.x
is replaced bycore_version_requirement: ^8
, the module is not installable on earlier Drupal versions.Drupal.behaviors.visualization_gva = { - attach: function(context) { - $.each($(".visualization-chart-gva", context).not(".visualization-processed"), function(idx, value) { + attach: function (context) { + $.each($(".visualization-chart-gva", context).not(".visualization-processed"), function (idx, value) { var chart_id = $(value).attr("id"); var chart = drupalSettings.visualization[chart_id];
The report shown in the issue summary does not show any issue for JavaScript files. Either the shown report is not complete, or those changes are off-topic.
- Assigned to sahil.goyal
- ๐ฎ๐ณIndia sahil.goyal
addressed all the suggestive coding standards and warnings in #16.
only last suggestion is left,
As here task is to address PHPCS issues, which are related to the PHP code standards used in the project.Regarding the JavaScript files, the report shown in the issue summary does not indicate any issues related to those files. Therefore, consider those changes as off-topic for current task of addressing PHPCS issues.
Please review.Thanks @apaderno for addressing the issues.
- Status changed to Needs review
over 1 year ago 11:18am 4 April 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
I ran
phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml ./visualization
and I got a longer report than what previously shown.I updated the issue summary.
Please, when opening these issues, show the full report, not only part of it as it is more convenient for the user who is going to provide a patch or create a MR.
- Status changed to Needs work
over 1 year ago 11:22am 4 April 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
Let's start with those issues. I would leave the ones about JavaScript code for a different issue.
- ๐ฎ๐ณIndia sahil.goyal
Hi @apaderno, when i run
phpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml ./visualization
i can only seen a single issue, not a list of as updated in IS. Attaching Sreenshot. - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
Cloning the 8.x-1.x branch with
git clone --branch '8.x-1.x' https://git.drupalcode.org/project/visualization.git ./visualization
and then executingphpcs --standard="Drupal,DrupalPractice" --extensions=php,module,inc,install,test,profile,theme,info,txt,md,yml ./visualization
gives the report shown in the issue summary. I tried again, and the report is not just for a single line. - First commit to issue fork.
- First commit to issue fork.
- Issue was unassigned.
- First commit to issue fork.
- Status changed to Needs review
11 months ago 12:38pm 27 December 2023 - ๐ฎ๐ณIndia dev16.addweb
silvi.addweb โ made their first commit to this issueโs fork.
- Status changed to Needs work
3 months ago 11:22pm 13 August 2024 Hi @sakthi_dev & @silvi.addweb,
The changes you made on MR !3 was applied smoothly, however, one error still reported, please see below:
visualization git:(8.x-1.x) curl https://git.drupalcode.org/project/visualization/-/merge_requests/3.diff | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 32383 0 32383 0 0 36151 0 --:--:-- --:--:-- --:--:-- 36590 patching file js/gva.js patching file js/highcharts.js patching file js/visualization.js patching file src/Form/VisualizationSettingsForm.php patching file src/Plugin/VisualizationHandlerManager.php patching file src/Plugin/views/style/Visualization.php patching file src/Plugin/visualization/handler/GoogleVisualizationAPIHandler.php patching file src/Plugin/visualization/handler/HighchartsHandler.php patching file src/VisualizationHandlerInterface.php patching file visualization.api.php patching file visualization.info.yml patching file visualization.links.menu.yml patching file visualization.module patching file visualization.theme.inc โ visualization git:(8.x-1.x) โ cd .. โ contrib git:(main) โ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig visualization FILE: ...web/modules/contrib/visualization/src/Plugin/views/style/Visualization.php -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 175 | ERROR | Public method name "Visualization::build_sort" is not in | | lowerCamel format -------------------------------------------------------------------------------- Time: 279ms; Memory: 10MB
Kindly check
Thanks,
Jake- Merge request !4Created a new merge request to get the list of the PHP_CodeSniffer errors/warnings to fix โ (Open) created by apaderno