- Issue created by @mukesh88
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 9:42am 5 April 2023 - Status changed to Needs work
over 1 year ago 10:15am 5 April 2023 - 🇮🇳India TanujJain-TJ
Tested and verified patch #2 on drupal:10.1.x, the patch applied successfully but still getting some PHPCS errors after running
phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml
need to fix below mentioned errors, chaning it to needs work again.
FILE: /entity_print/src/Asset/AssetCollector.php -------------------------------------------------------------------------------- FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES -------------------------------------------------------------------------------- 51 | WARNING | \Drupal calls should be avoided in classes, use dependency | | injection instead 52 | WARNING | The removal-version 'entity_print:3.0.0' does not match the | | lower-case machine-name standard: drupal:n.n.n or | | project:n.x-n.n -------------------------------------------------------------------------------- FILE: entity_print/src/Asset/AssetRenderer.php -------------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE -------------------------------------------------------------------------- 71 | WARNING | Variable $build inside unset call is undefined. --------------------------------------------------------------------------
- Assigned to sakthi_dev
- Issue was unassigned.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
-
+++ b/src/PrintBuilder.php @@ -42,11 +49,14 @@ class PrintBuilder implements PrintBuilderInterface { $this->stringTranslation = $string_translation; + $this->fileSystem = $file_system;
We need a deprecation layer here, so default this to null, and trigger_error if the value is null, then fetch from the singleton
See
\Drupal\entity_print\Asset\AssetCollector::__construct
for an exampleWill need a new change record to go with that, so you can use it in the message. The link to create a change record is at the top of this page
-
+++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestPrintEngine.php @@ -65,8 +65,18 @@ class TestPrintEngine extends PrintEngineBase { + $this->configuration['test_engine_setting'] = $form_state->getValue( + [ + 'testprintengine', + 'test_engine_setting', + ] + ); + $this->configuration['test_engine_suffix'] = $form_state->getValue( + [ + 'testprintengine', + 'test_engine_suffix', + ] + ); +++ b/tests/modules/entity_print_test/src/Plugin/EntityPrint/PrintEngine/TestWordPrintEngine.php @@ -74,7 +74,12 @@ class TestWordPrintEngine extends PrintEngineBase { - $this->configuration['test_word_setting'] = $form_state->getValue(['test_word_print_engine', 'test_word_setting']); + $this->configuration['test_word_setting'] = $form_state->getValue( + [ + 'test_word_print_engine', + 'test_word_setting', + ] + ); +++ b/tests/src/FunctionalJavascript/EntityPrintAdminTest.php @@ -101,7 +101,13 @@ class EntityPrintAdminTest extends WebDriverTestBase { - $this->assertEquals(['test_engine_setting' => 'testvalue', 'test_engine_suffix' => 'overridden'], $config_entity->getSettings()); + $this->assertEquals( + [ + 'test_engine_setting' => 'testvalue', + 'test_engine_suffix' => 'overridden', + ], + $config_entity->getSettings() + );
can we collapse these a bit, the [] can go on the same lines as the ()
The items at #4 are false positives and need to be ignored by adding some ignore rules per https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignorin...
-
@larowlan #7 point 2 have done with added phpcs ignore and recreate patch. I checked for point 1
\Drupal\entity_print\Asset\AssetCollector::__construct
object is InfoParserInterface then @trigger_error for deprecation so in PrintBuilder class which object will check null for @trigger_error.
@larowlan could you please check the below code for #7 point 1
+ * @param \Drupal\Core\File\FileSystemInterface $file_system + * The file system service. */ - public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation) { + public function __construct(RendererFactoryInterface $renderer_factory, EventDispatcherInterface $event_dispatcher, TranslationInterface $string_translation, FileSystemInterface $file_system = NULL) { $this->rendererFactory = $renderer_factory; $this->dispatcher = $event_dispatcher; $this->stringTranslation = $string_translation; + if (!$file_system) { + @trigger_error('Calling $file_system without argument the constructor of ' . __CLASS__ . ' is deprecated in entity_print:8.x-2.4 and is removed from entity_print:3.0.0. Pass an instance of FileSystemInterface instead. See https://www.drupal.org/node/3238430', E_USER_DEPRECATED); + $this->fileSystem = \Drupal::service('file_system'); + } + $this->fileSystem = $file_system;
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Ah sorry, that was a bad example, because it was dealing with a replaced paramater.
Can you check
\Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor::__construct
in core for a better example @larowlan I checked file core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php it isn't good example for comment #11
/** * Constructs an AjaxResponseAttachmentsProcessor object. * * @param \Drupal\Core\Asset\AssetResolverInterface $asset_resolver * An asset resolver. * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * A config factory for retrieving required config objects. * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $css_collection_renderer * The CSS asset collection renderer. * @param \Drupal\Core\Asset\AssetCollectionRendererInterface $js_collection_renderer * The JS asset collection renderer. * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. * @param \Drupal\Core\Render\RendererInterface $renderer * The renderer. * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler * The module handler. */ public function __construct(AssetResolverInterface $asset_resolver, ConfigFactoryInterface $config_factory, AssetCollectionRendererInterface $css_collection_renderer, AssetCollectionRendererInterface $js_collection_renderer, RequestStack $request_stack, RendererInterface $renderer, ModuleHandlerInterface $module_handler) { $this->assetResolver = $asset_resolver; $this->config = $config_factory->get('system.performance'); $this->cssCollectionRenderer = $css_collection_renderer; $this->jsCollectionRenderer = $js_collection_renderer; $this->requestStack = $request_stack; $this->renderer = $renderer; $this->moduleHandler = $module_handler; }
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Sorry, you'll need to check it in 10.1.x
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
@mukesh88 any updates here? Anything I can help with - feel free to ping me on slack if you need more info
@larowlan Thanks for response. Actually i am busy on project i don't spent much time on contribution if i will free or sure will ping you on slack
- First commit to issue fork.
- Assigned to akshaydalvi212
- @akshaydalvi212 opened merge request.
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 10:28am 13 April 2023 - 🇮🇳India akshaydalvi212
removed all the reported issues with PHPCS, and created the MR for the same.
kindly review. - 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
@akshaydalvi212 that merge request contains lots of changes that aren't relevant to PHPCS.
I've closed it. We have a patch above, with actionable items. Let's work on top of this patch, or use it as the basis for an MR.
- Status changed to Needs work
over 1 year ago 11:25am 26 April 2023 - First commit to issue fork.
- last update
over 1 year ago 38 pass, 18 fail - @bharath-kondeti opened merge request.
- Status changed to Needs review
over 1 year ago 11:35am 26 April 2023 - 🇮🇳India bharath-kondeti Hyderabad
https://git.drupalcode.org/project/entity_print/-/merge_requests/39 Raised a MR with the fixes. Please review. Thanks
- Status changed to Needs work
over 1 year ago 7:14am 30 May 2023 Hii,
patch applied cleanly for #26 but still there are some errors.\entity_print\modules\entity_print_views\src\Controller\ViewPrintController.php
----------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------\entity_print\src\Asset\AssetCollector.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Controller\EntityPrintController.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Entity\PrintEngineStorageInterface.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\EntityPrintPermissions.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Event\PrintEventBase.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\EventSubscriber\PrintEngineExceptionSubscriber.php
-----------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
-----------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------\entity_print\src\Form\SettingsForm.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Plugin\EntityPrint\PrintEngine\DomPdf.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Plugin\EntityPrintPluginManager.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Plugin\PrintEngineInterface.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\PrintBuilder.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------\entity_print\src\Renderer\RendererBase.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------- Status changed to Needs review
10 months ago 10:20am 17 January 2024 - last update
10 months ago 23 pass, 26 fail The last submitted patch, 28: Fixed-the-issues-reported-by-phpcs-27.patch, failed testing. View results →
- Status changed to Needs review
10 months ago 11:14am 17 January 2024 - last update
10 months ago Patch Failed to Apply - 🇮🇳India viren18febS
i am not able to apply patch #28 , so i have applied patch #26 and its applied.
after this patch i have found some more issues so i have added a patch for the fixes. please review. - Status changed to Needs work
10 months ago 7:47am 31 January 2024 Hi, I am not able to apply patch #30 it is throwing error.
sha@LAPTOP-6VFKUABC MINGW64 ~/entity_print (8.x-2.x) $ git apply issues-3352346-Fixed-error-reported-by-phpcs.patch error: patch failed: src/Asset/AssetCollector.php:5 error: src/Asset/AssetCollector.php: patch does not apply error: patch failed: src/Plugin/EntityPrint/PrintEngine/DomPdf.php:2 error: src/Plugin/EntityPrint/PrintEngine/DomPdf.php: patch does not apply
So changing the status to Needs work.
Thankyou.- 🇮🇳India Yashaswi18
Hello, I applied the patch for the version 8.x-2.x, the patch applied succesfully.
Checking patch modules/entity_print_views/src/Controller/ViewPrintController.php... Checking patch src/Asset/AssetCollector.php... Checking patch src/Controller/EntityPrintController.php... Checking patch src/Entity/PrintEngineStorageInterface.php... Checking patch src/EntityPrintPermissions.php... Checking patch src/Event/PrintEventBase.php... Checking patch src/EventSubscriber/PrintEngineExceptionSubscriber.php... Checking patch src/Form/SettingsForm.php... Checking patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php... Checking patch src/Plugin/EntityPrintPluginManager.php... Checking patch src/Plugin/PrintEngineInterface.php... Checking patch src/PrintBuilder.php... Checking patch src/Renderer/RendererBase.php... Applied patch modules/entity_print_views/src/Controller/ViewPrintController.php cleanly. Applied patch src/Asset/AssetCollector.php cleanly. Applied patch src/Controller/EntityPrintController.php cleanly. Applied patch src/Entity/PrintEngineStorageInterface.php cleanly. Applied patch src/EntityPrintPermissions.php cleanly. Applied patch src/Event/PrintEventBase.php cleanly. Applied patch src/EventSubscriber/PrintEngineExceptionSubscriber.php cleanly. Applied patch src/Form/SettingsForm.php cleanly. Applied patch src/Plugin/EntityPrint/PrintEngine/DomPdf.php cleanly. Applied patch src/Plugin/EntityPrintPluginManager.php cleanly. Applied patch src/Plugin/PrintEngineInterface.php cleanly. Applied patch src/PrintBuilder.php cleanly. Applied patch src/Renderer/RendererBase.php cleanly.
Ran the command phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml entity_print/ after applying patch, there are no phpcs errors left.
- Status changed to Needs review
10 months ago 1:21pm 1 February 2024 - last update
10 months ago Patch Failed to Apply - Status changed to Needs work
5 months ago 11:04pm 11 June 2024 - 🇺🇸United States smustgrave
If the maintainers want to keep this type of issue then probably should be rescoped to cover the warnings from gitlab.
Personal recommendation would be to close this out and start a new one since these type of tickets come off as farming.