- Issue created by @indrapatil
- ๐ฎ๐ณIndia Shreya_98
I had fixed all the errors reported by phpcs but some warning are still left . I have attached screenshot of warnings which will be solve by maintainer.
- Issue was unassigned.
- Status changed to Needs review
about 1 year ago 7:59am 28 August 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
I am resetting the Assigned field, since that person did not do anything for this issue after more than 24 hours.
- Status changed to Needs work
about 1 year ago 8:12am 28 August 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
@@ -12,20 +12,23 @@ The popin content is composed of :
Since the file is edited, the spaces before colons must be removed too.
+The popin will be displayed once per user session. Update the popin + configuration form will reset this setting. ## Installation
There must be two empty lines before a new section.
-Finally you can configure the popin content and visibility on /admin/content/popin. +After that, head to blocks structure configuration to add the รยซ Popin block รยป + to a region (footer for example).
In English, text is quoted between double quote characters (").
+(You can use block visibility settings if you want to display + the popin only on the frontpage)
A period is missing after frontpage.
+/** + * @file + * Contains implementation of theme hooks for the Popin module. + */ +
The usual description is Hook implementations for the [module name] module. where [module name] is replaced by the module name given in the .info.yml file.
/** - * @var FileUsageInterface + * The file usage service for tracking how files are used within Drupal. + * + * @var \Drupal\file\Entity\FileUsageInterface */ protected $fileUsage;
The file usage service. is sufficient.
/** - * @var AccountProxyInterface + * The current user account proxy service. + * + * @var \Drupal\Core\Session\AccountProxyInterface */
The current user. is sufficient.
+ /** + * Creates a new instance of the Popin configuration form. + * + * @param \Symfony\Component\DependencyInjection\ContainerInterface $container + * The service container. + * + * @return \Drupal\your_module\PopinConfigForm + * A new instance of the PopinConfigForm class. + */
The documentation comment for methods inherited from a parent class or defined in an interface the class implements is
{@inheritdoc}
./** + * The configuration factory service for managing configuration settings. + * * @var \Drupal\Core\Config\ConfigFactoryInterface */ protected $configFactory;
The configuration factory. is sufficient.
- Assigned to nitin_lama
- ๐ฎ๐ณIndia nitin_lama India
Addressed comment #4. Providing updated patch.
- Issue was unassigned.
- Status changed to Needs review
about 1 year ago 7:33am 29 August 2023 - Status changed to Needs work
about 1 year ago 5:14pm 29 August 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
## Installation You can download this module with composer, and enable it like any other module. -After that, head to blocks structure configuration to add the รยซ Popin block รยป to a region (footer for example). - -(You can use block visibility settings if you want to display the popin only on the frontpage) - -Finally you can configure the popin content and visibility on /admin/content/popin. +After that, head to blocks structure configuration to add the "Popin block" to a +region (footer for example).
The Installation section should say what the template suggests.
Install as you would normally install a contributed Drupal module. For further information, see [Installing Drupal Modules](https://www.drupal.org/docs/extending-drupal/installing-drupal-modules).
The rest is part of the Configuration section.
The README.md file is not even complete. It would be better to leave any change for that file to a different issue.
-Finally you can configure the popin content and visibility on /admin/content/popin. +After that, head to blocks structure configuration to add the "Popin block" to a +region (footer for example). +(You can use block visibility settings if you want to display the popin only on +the frontpage).
SInce that is a parenthetical, the period goes inside the parentheses.
- First commit to issue fork.
- First commit to issue fork.
Hi,
Reviewed #10, the patch is not applying and is throwing following error.Checking patch README.md...
error: while searching for:All are optionals.
You can customize the fields order by editing the provided template (block-popin.html.twig).
You can configure the visibility of the popin by date, or globally.
The popin will be displayed once per user session. Update the popin configuration form will reset this setting.
## Installation
You can download this module with composer, and enable it like any other module.
After that, head to blocks structure configuration to add the ยซ Popin block ยป to a region (footer for example).
(You can use block visibility settings if you want to display the popin only on the frontpage)
Finally you can configure the popin content and visibility on /admin/content/popin.
error: patch failed: README.md:12
error: README.md: patch does not apply
Checking patch css/popin.css...
error: while searching for:
margin-top: 15px;
text-align: center;
}error: patch failed: css/popin.css:76
error: css/popin.css: patch does not apply
Checking patch popin.module...
error: while searching for:
<?php/**
* Implements hook_theme().
*/error: patch failed: popin.module:1
error: popin.module: patch does not apply
Checking patch src/Form/PopinConfigForm.php...
error: while searching for:
namespace Drupal\popin\Form;use Drupal\Core\Cache\Cache;
use Drupal\Core\Datetime\DrupalDateTime;
use Drupal\Core\Form\ConfigFormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\file\Entity\File;
use Drupal\file\FileUsage\FileUsageInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\Config\ConfigManagerInterface;/**
* Class PopinConfigForm.
*/
class PopinConfigForm extends ConfigFormBase {/**
* @var FileUsageInterface
*/
protected $fileUsage;/**
* @var AccountProxyInterface
*/
protected $currentUser;error: patch failed: src/Form/PopinConfigForm.php:3
error: src/Form/PopinConfigForm.php: patch does not apply
Checking patch src/Plugin/Block/PopinBlock.php...
error: while searching for:
use Drupal\Core\Block\BlockBase;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Datetime\DrupalDateTime;
use Drupal\file\Entity\File;
use Drupal\image\Entity\ImageStyle;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Session\SessionManagerInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Symfony\Component\HttpFoundation\RequestStack;/**
* Provides a 'PopinBlock' block.error: patch failed: src/Plugin/Block/PopinBlock.php:5
error: src/Plugin/Block/PopinBlock.php: patch does not apply- Assigned to mohd sahzad
- Issue was unassigned.
- ๐ฎ๐ณIndia zkhan.aamir
Hi,
Patch #10 applied successfully.
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/popin (2.0.x) $ curl https://www.drupal.org/files/issues/2023-11-29/phpcs-fixes-3373278-10.patch | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 8006 100 8006 0 0 21012 0 --:--:-- --:--:-- --:--:-- 21124 patching file README.md patching file css/popin.css patching file popin.module patching file src/Form/PopinConfigForm.php patching file src/Plugin/Block/PopinBlock.php
No issues remianing.
Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules/popin (2.0.x) $ cd ../ Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules $ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,info,txt,md,css,js,yml popin/ Admin@DESKTOP-252TO6V MINGW64 ~/Desktop/projects/drupal/web/modules $
- Assigned to nitin_lama
- Issue was unassigned.
- Status changed to Needs review
9 months ago 4:59am 27 February 2024 - ๐ฎ๐ณIndia Yashaswi18
Hello, the patch #19 applies cleanly and there are no errors or warnings remaining when applied without checking out to any branch.
git apply phpcs-fixes-3373278-17.patch -v Checking patch README.md... Checking patch css/popin.css... Checking patch popin.module... Checking patch src/Form/PopinConfigForm.php... Checking patch src/Plugin/Block/PopinBlock.php... Applied patch README.md cleanly. Applied patch css/popin.css cleanly. Applied patch popin.module cleanly. Applied patch src/Form/PopinConfigForm.php cleanly. Applied patch src/Plugin/Block/PopinBlock.php cleanly.
- Status changed to Needs work
6 months ago 10:04am 23 May 2024 Hi,
Applied patch #17 and found one error
FILE: ...etInterns/Demo-site/drupal-orgissue/web/modules/contrib/popin/popin.module -------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------- 1 | ERROR | [x] Missing file doc comment -------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------- Time: 237ms; Memory: 10MB
- ๐ฎ๐ณIndia dev16.addweb
Hello,
I resolve all listed errors in this patch, please review.FILE: popin/css/popin.css --------------------------------------------------------------------------------------------------- FOUND 2 ERRORS AFFECTING 2 LINES --------------------------------------------------------------------------------------------------- 78 | ERROR | [x] Expected 1 newline at end of file; 3 found 80 | ERROR | [x] Additional whitespace found at end of file --------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY --------------------------------------------------------------------------------------------------- FILE: popin/popin.module -------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------- 1 | ERROR | [x] Missing file doc comment -------------------------------------------------------------------------------------------------- PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY -------------------------------------------------------------------------------------------------- FILE: popin/popin.module ---------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ---------------------------------------------------------------------- 3 | ERROR | Missing short description in doc comment ----------------------------------------------------------------------
- Status changed to RTBC
6 months ago 7:08am 30 May 2024 Hi @silvi.addweb,
Applied patch #22, confirmed all errors/warnings fixed.
popin git:(2.0.x) โ curl https://www.drupal.org/files/issues/2024-05-30/phpcs-fixes-3373278-22.patch | patch -p1 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 448 100 448 0 0 1461 0 --:--:-- --:--:-- --:--:-- 1503 patching file css/popin.css patching file popin.module โ popin git:(2.0.x) โ cd .. โ contrib git:(main) โ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig popin โ contrib git:(main) โ
Will move this to RTBC.
Thanks,
Jake- Status changed to Needs work
6 months ago 8:34am 30 May 2024 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
Since GitLab CI has been enabled for this project, and since there have been commits in the project that fixed some of the reported PHP_Codesniffer warnings/errors, a new MR must be provided. In fact, a single error still needs to be fixed.
- Status changed to Needs review
6 months ago 8:37am 30 May 2024 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
-
Andrii Momotov โ
committed a00ddbd5 on 2.0.x authored by
apaderno โ
Issue #3373278: Fix the issues reported by phpcs
-
Andrii Momotov โ
committed a00ddbd5 on 2.0.x authored by
apaderno โ
- Status changed to Fixed
5 months ago 4:49pm 29 June 2024 - Status changed to Fixed
5 months ago 4:53pm 29 June 2024