Fix the errors/warnings reported by PHP_CodeSniffer

Thanks for this patch @mohit.bansal623 but it is simply too big to review. As you can see @MegaChriz has been addressing this issue in bite-sized pieces. Can I suggest that you supply a patch for just a few files, or even just one if the changes are large? As a guide let's try to keep the patches below 50KB.

I tried reducing the patch size @darvanen. Uploading a patch for some files.

Hi, I worked on the patch, uploading it, please review it.

Hi, i will review this last patch.

I found some erros and i cant apply this last patch. So i will unassigned and wait for the updates patch

Hi @Guilherme Rabelo and welcome to the Drupal community :)

Thanks for reviewing the patch, to make the most of the time you spent doing so, any information you can provide on the problems you encountered will help those working on it be able to fix it - or potentially help you with it.

For instance: I'd like to check that you were applying the patch to the 8.x -3.x-dev version of the module, and that you used the -p1 flag on the patch command from within the modules/feeds/ folder?

These issues can be tricky because they touch code potentially modified by other patches.

Here is the current output when trying to apply the patch.

100 22875  100 22875    0     0   194k      0 --:--:-- --:--:-- --:--:--  208k
patching file src/Component/CsvParser.php
patching file src/Entity/Feed.php
Hunk #1 succeeded at 438 (offset 1 line).
patching file src/EventSubscriber/LazySubscriber.php
patching file src/Exception/FeedsRuntimeException.php
patching file src/Feeds/Parser/OpmlParser.php
Hunk #1 FAILED at 70.
1 out of 1 hunk FAILED -- saving rejects to file src/Feeds/Parser/OpmlParser.php.rej
patching file src/Feeds/State/CleanState.php
patching file src/Feeds/State/CleanStateInterface.php
patching file src/FeedsExecutable.php
patching file src/FeedsQueueExecutable.php
patching file src/Plugin/Field/FieldFormatter/FeedsItemGuidFormatter.php
patching file src/Plugin/Field/FieldFormatter/FeedsItemUrlFormatter.php
patching file tests/src/Functional/PrivateFileTest.php
patching file tests/src/Kernel/Entity/FeedTest.php
Hunk #2 succeeded at 298 (offset 10 lines).
patching file tests/src/Traits/FeedsReflectionTrait.php
patching file tests/src/Unit/Component/CsvParserTest.php
patching file tests/src/Unit/Feeds/Fetcher/DirectoryFetcherTest.php
patching file tests/src/Unit/Feeds/Fetcher/HttpFetcherTest.php
patching file tests/src/Unit/Feeds/Fetcher/UploadFetcherTest.php
patching file tests/src/Unit/Feeds/Parser/SitemapParserTest.php
Hunk #1 succeeded at 8 (offset 1 line).
Hunk #2 succeeded at 85 (offset 1 line).
patching file tests/src/Unit/Feeds/Parser/SyndicationParserTest.php
Reversed (or previously applied) patch detected!  Assume -R? [n] n
Apply anyway? [n] n
Skipping patch.
2 out of 2 hunks ignored -- saving rejects to file tests/src/Unit/Feeds/Parser/SyndicationParserTest.php.rej
patching file tests/src/Unit/Plugin/QueueWorker/FeedQueueWorkerBaseTest.php
patching file tests/src/Unit/Plugin/QueueWorker/FeedRefreshTest.php
patching file tests/src/Unit/Result/FetcherResultTest.php

Where a couple failed to apply.

And here are some of the remaining changes:

❯ phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml .

FILE: /Users/joel/Contrib/feeds/feeds.install
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 24 | ERROR | Empty installation hooks are not necessary
----------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Unit/Component/XmlParserTraitTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 11 ERRORS AFFECTING 11 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 58 | ERROR | The array declaration extends to column 98 (the limit is 80). The array content should be split up over multiple lines
 59 | ERROR | The array declaration extends to column 106 (the limit is 80). The array content should be split up over multiple lines
 60 | ERROR | The array declaration extends to column 110 (the limit is 80). The array content should be split up over multiple lines
 61 | ERROR | The array declaration extends to column 134 (the limit is 80). The array content should be split up over multiple lines
 62 | ERROR | The array declaration extends to column 118 (the limit is 80). The array content should be split up over multiple lines
 63 | ERROR | The array declaration extends to column 118 (the limit is 80). The array content should be split up over multiple lines
 64 | ERROR | The array declaration extends to column 122 (the limit is 80). The array content should be split up over multiple lines
 69 | ERROR | The array declaration extends to column 114 (the limit is 80). The array content should be split up over multiple lines
 72 | ERROR | The array declaration extends to column 166 (the limit is 80). The array content should be split up over multiple lines
 75 | ERROR | The array declaration extends to column 166 (the limit is 80). The array content should be split up over multiple lines
 78 | ERROR | The array declaration extends to column 138 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Unit/Component/CsvParserTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 58 | ERROR | The array declaration extends to column 81 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Unit/Feeds/Parser/Form/CsvParserFeedFormTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 30 | ERROR | The array declaration extends to column 82 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemUrlFormatterTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 60 | ERROR | The array declaration extends to column 177 (the limit is 80). The array content should be split up over multiple lines
 61 | ERROR | The array declaration extends to column 190 (the limit is 80). The array content should be split up over multiple lines
 62 | ERROR | The array declaration extends to column 81 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemGuidFormatterTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 67 | ERROR | The array declaration extends to column 221 (the limit is 80). The array content should be split up over multiple lines
 68 | ERROR | The array declaration extends to column 189 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/Plugin/Field/FieldFormatter/FeedsItemImportedFormatterTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 82 | ERROR | The array declaration extends to column 93 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/CronTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 95 | ERROR | The array declaration extends to column 108 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/Feeds/Parser/Form/CsvParserFeedFormTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 28 | ERROR | The array declaration extends to column 88 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Functional/UpdateNonExistentTest.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------
 312 | ERROR | The array declaration extends to column 101 (the limit is 80). The array content should be split up over multiple lines
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Kernel/SkipNewTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 33 | ERROR | The array declaration extends to column 114 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/tests/src/Kernel/FeedsEventsTest.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------------------------------------------------------------
 117 | ERROR | The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
 139 | ERROR | The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
 180 | ERROR | The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
 222 | ERROR | The array declaration extends to column 83 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Form/FeedScheduleImportForm.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 56 | ERROR | The array declaration extends to column 94 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Form/CustomSourceDeleteForm.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 90 | ERROR | [x] Each PHP statement must be on a line by itself
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Form/FeedDeleteForm.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------------------------------
 58 | ERROR | The array declaration extends to column 96 (the limit is 80). The array content should be split up over multiple lines
 64 | ERROR | The array declaration extends to column 86 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Form/FeedUnlockForm.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
 39 | ERROR | The array declaration extends to column 94 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Zend/Extension/Georss/Entry.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | The text '@deprecated in favor of \Drupal\feeds\Laminas\Extension\Georss\Entry. Use that instead.' does not match the standard format: @deprecated in
    |       | %deprecation-version% and is removed from %removal-version%. %extra-info%.
 10 | ERROR | Each @deprecated tag must have a @see tag immediately following it
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Plugin/Type/FeedsAnnotationFactory.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 10 | ERROR | The text '@deprecated in Feeds 8.x-3.0-alpha6, will be removed before Feeds 8.x-3.0.' does not match the standard format: @deprecated in %deprecation-version%
    |       | and is removed from %removal-version%. %extra-info%.
 10 | ERROR | Each @deprecated tag must have a @see tag immediately following it
 24 | ERROR | The trigger_error message 'FeedsAnnotationFactory is deprecated in Feeds 8.x-3.0-alpha6 and will be removed before Feeds 8.x-3.0. Implement
    |       | \Drupal\Core\Plugin\ContainerFactoryPluginInterface instead.' does not match the relaxed standard format: %thing% is deprecated in %deprecation-version% any free
    |       | text %removal-version%. %extra-info%. See %cr-link%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Component/XmlParserTrait.php
----------------------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
----------------------------------------------------------------------------------------------------------------
 15 | WARNING | Property name "$_elementRegex" should not be prefixed with an underscore to indicate visibility
 15 | ERROR   | Class property $_elementRegex should use lowerCamel naming without underscores
 22 | WARNING | Property name "$_useError" should not be prefixed with an underscore to indicate visibility
 22 | ERROR   | Class property $_useError should use lowerCamel naming without underscores
 29 | WARNING | Property name "$_entityLoader" should not be prefixed with an underscore to indicate visibility
 29 | ERROR   | Class property $_entityLoader should use lowerCamel naming without underscores
 36 | WARNING | Property name "$_errors" should not be prefixed with an underscore to indicate visibility
 36 | ERROR   | Class property $_errors should use lowerCamel naming without underscores
----------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Annotation/FeedsTarget.php
-------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------
 26 | ERROR | Class property $field_types should use lowerCamel naming without underscores
-------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Annotation/FeedsSource.php
-------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------
 26 | ERROR | Class property $field_types should use lowerCamel naming without underscores
-------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/TargetDefinition.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 148 | ERROR | The array declaration extends to column 85 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Feeds/Target/EntityReference.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------
 159 | ERROR | The array declaration extends to column 85 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Feeds/Target/UserRole.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------
 120 | ERROR | The array declaration extends to column 108 (the limit is 80). The array content should be split up over multiple lines
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Feeds/Item/SitemapItem.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
 10 | ERROR | Missing member variable doc comment
 11 | ERROR | Missing member variable doc comment
 12 | ERROR | Missing member variable doc comment
 13 | ERROR | Missing member variable doc comment
----------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Feeds/Item/OpmlItem.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
 10 | ERROR | Missing member variable doc comment
 11 | ERROR | Missing member variable doc comment
 12 | ERROR | Missing member variable doc comment
 13 | ERROR | Missing member variable doc comment
----------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Feeds/Item/SyndicationItem.php
-------------------------------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 11 LINES
-------------------------------------------------------------------------------------------
 10 | ERROR | Missing member variable doc comment
 11 | ERROR | Missing member variable doc comment
 12 | ERROR | Missing member variable doc comment
 13 | ERROR | Class property $author_name should use lowerCamel naming without underscores
 13 | ERROR | Missing member variable doc comment
 14 | ERROR | Missing member variable doc comment
 15 | ERROR | Missing member variable doc comment
 16 | ERROR | Missing member variable doc comment
 17 | ERROR | Missing member variable doc comment
 18 | ERROR | Missing member variable doc comment
 19 | ERROR | Missing member variable doc comment
 20 | ERROR | Missing member variable doc comment
-------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Commands/FeedsDrushCommands.php
---------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
---------------------------------------------------------------------------------------------------------------------------------------
 142 | ERROR | The array declaration extends to column 141 (the limit is 80). The array content should be split up over multiple lines
 180 | ERROR | The array declaration extends to column 142 (the limit is 80). The array content should be split up over multiple lines
 224 | ERROR | The array declaration extends to column 144 (the limit is 80). The array content should be split up over multiple lines
 323 | ERROR | The array declaration extends to column 140 (the limit is 80). The array content should be split up over multiple lines
 357 | ERROR | The array declaration extends to column 142 (the limit is 80). The array content should be split up over multiple lines
---------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/FeedTypeAccessControlHandler.php
----------------------------------------------------------------------------------
FOUND 2 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------------------
 28 | WARNING | [ ] Code after the RETURN statement on line 26 cannot be executed
 28 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 8
 38 | WARNING | [ ] Code after the RETURN statement on line 36 cannot be executed
 38 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 8
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/FeedForm.php
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------------------------------------------------------------
  64 | ERROR | The array declaration extends to column 92 (the limit is 80). The array content should be split up over multiple lines
 157 | ERROR | The array declaration extends to column 90 (the limit is 80). The array content should be split up over multiple lines
 191 | ERROR | The array declaration extends to column 92 (the limit is 80). The array content should be split up over multiple lines
--------------------------------------------------------------------------------------------------------------------------------------


FILE: /Users/joel/Contrib/feeds/src/Event/EventDispatcherTrait.php
-------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------
 24 | WARNING | Property name "$_eventDispatcher" should not be prefixed with an underscore to indicate visibility
 24 | ERROR   | Class property $_eventDispatcher should use lowerCamel naming without underscores
-------------------------------------------------------------------------------------------------------------------

Time: 2.96 secs; Memory: 20MB

But note some changes are API changes when renaming things for example so we can't make those changes easily.

I rerolled the #33 patch and after that i was able to apply the patch. Now i will work on the issues to fixe.

I cound't apply #33 patch.
So i rerolled the module from 8.x-3.x and after that i can run the phpcs and shows me a lot of issues about coding standadrs. The problems are in issuesPatch-41.txt file. So i fixed almost of the issues. Still have some unused variable problems, that i think its more interesting to someone who knows more about this module fixe this issues.

Needs to review this patch.

My 2c where there was a question or similar opening:

#42.0: "This is a big patch" - I think contributers would have more luck getting a patch through if you focussed on a single file at a time as I've previously encouraged.

#42.1: This is not a coding standards change, the code standards allow for either snake or camel case. I agree they should not be altered.

#42.3: I don't mind this change on a Do-While, the comment is inside the brackets and therefore *to me* makes sense to be indented. If it's bothersome perhaps consider changing to a While loop instead?

#42.6: There are return commands so the breaks are inaccessible, it's fine to remove them.

#42.9: I agree that feeds:feeds is the right name to use.

I agree with all the points I did not mention.

#42.1: This is not a coding standards change, the code standards allow for either snake or camel case. I agree they should not be altered.

That is true for Drupal 8, but we are running Drupal 9 and trying to update some custom code that extends SyndicationItem to be in conformance with PHPCS Drupal. But we are stuck with snake case-related errors until this patch gets merged.

Maybe this requires a major version increment to indicate that it is backwards incompatible?

I'll work on it :)

I worked on #41's patch to correct some Dependency Injection issues which were causing the module to break when adding new feeds.

Also worked on FeedsAnnotationFactory to correct the trigger_error message, referencing it to #3136615: Remove deprecated FeedsAnnotationFactory class as to match the relaxed standard format.

Most "unusable variables" weren't touched, as it's more suitable for someone with more knowledge about the module to check.

Note that the content inside [ ] on each patch references to the folder where i worked on for that single patch.

Feed patch was also sent to #3248925-11: Dependency injection for \Drupal calls in classes correcting dependency injection issues.

Hi MegaChriz, thanks for the feedback.

2939688-47[src].patch

• It correts Undefined method 't' on the getLabel method's return. I'll be correcting this one to reflect your sample.

2939688-47[Plugin].patch

• For this one, the relaxed standard for @trigger_error only allows the standard project:n.x-n.n. With -alpha6 it only displays a warning (which i think is not a problem). It also requires a %cr-link%, that's why I included the link to #3136615. Should i keep the link to the issue and edit this to reflect -alpha6 as it only generates a warning?

2939688-47[Component].patch

• This one only puts #[\ReturnTypeWillChange] before the function comment block. Leaving it below the comment block generates an error on the comments. I just moved it above, but it's still declared right before the function itself with a special comment only for it :)

I'll double check #41 and try to apply what's stated on your feedback at #42, including keeping snake case variables to avoid breaking other contrib modules.

So, I'll be working on it :)

Hi @MegaChriz,

I worked on this issue following what you pointed in #42, #48 and #50.

Some points i didn’t touch:

#42.3 – I agree with @darvanen that

the comment is inside the brackets and therefore *to me* makes sense to be indented.

#42.5 – I couldn’t understand why $row is being overwritten here, so i’ll leave this one for someone with better understanding.

#42.6 – I agree with @darvanen that

There are return commands so the breaks are inaccessible, it's fine to remove them.

#42.8 – I couldn’t think of more useful descriptions for these properties, as they have a very simple context.

I also worked on every Dependency Injection that were missing on these files, but i’ll review it in details on #3248925: Dependency injection for \Drupal calls in classes

Every change was also tested on the UI to make sure nothing is breaking and that translations are working as intended.

Needs review :)

I'm sending a new patch to correct the DruschCommands kernel tests errors regarding the exit code (which i forgot to check before).

Adds const EXIT_ERROR = 1; and returns self::EXIT_ERROR; after each logger()->error() message to match the expected error code on Kernel Tests.

Im going to review it

@MegaChriz

I indeed diffed them against #41. I'll create the diffs against 8.x-3.x as pointed.

Thanks for the feedback :)

Working on it.

Ok, so i broke #41 patch into smaller pieces (to keep sanity) and worked on the content of the patches so that it applies against 8.x-3.x as pointed.

I tested everything again to make sure nothing is broken, and so far everything is ok.

Unused variables remains untouched.

Needs review :)

Silly me, forgot to change status and unassign :P

Corrects exit code for --import-disabled error message on [Commands] patch.

I couldn't fix the errors in [Feeds], [src] and [results] :/

Okay, so... I focused on studying and understanding the tests, and I realized that some changes, despite fixing the CS, were causing some strange errors not only in the tests, but in the module itself.

I then reworked all the patches (now with a better understanding of it), and I think this time it's ok for review.

As the number of patches was large in my last upload (14 patches) and most were just small changes, this time I split it into just 4 patches to keep things organized (sorry for cluttering the feed with too many patches :P).

Unused variables and #[\ReturnTypeWillChange] remains untouched (see sniffer.txt for details).

Needs review.

PS.: Failed tests due to addition of dependency injection of services on classes. Tests.patch contains all the changes made on those tests.

I'll work on it.

Hi @MegaChriz

I worked on everything that was pointed on #70.

I also reverted the changes to exceptions where i was passing a variable ($errorMessage) as pointed in #70.4 on other classes.

I split the bigger patch in 4 new patches with the naming <className>+Tests so that it's easier for you to check them :). Those will (for sure) have errors, but that's because they need the editions made in the test (Mocking of injected services).

Unused variables keep untouched and, as we didn't (yet) find a good way to translate exceptions, phpcs will keep warning about them :/

All that said, needs review. :)

Fixes EntityProcessorBase specifically requiring the mysql database. Changed to a gereric database requirement.

Fixes File service dependency injection of file.repository, provided that this service only exists from D9.3 onwards.

Needs review together with the other patches on #75 :)

Right! Sorry for the mess, I'm still learning how to (correctly) use drupal.org, but I'm really grateful for your feedback @MegaChriz

Great @MegaChriz, indeed i sent a lot of patches to this one hahaha

I checked the list and looks like all of them are included. Thanks!

Hi @MegaChriz,
reviewing your patch. Thanks!!!

Hi @MegaChriz,
I tested your patch. But patch test is failing.

SR COMPUTER@DESKTOP-CHNCDT9 MINGW64 /c/xampp/htdocs/Project/siteforcontributing/web/modules/contrib/feeds-2939688 (8.x-3.x)
$ git apply -v 2939688-80[EntityProcessorBase+Tests].patch
Checking patch src/Feeds/Processor/EntityProcessorBase.php...
error: while searching for:
      '@entity' => mb_strtolower($this->entityTypeLabel()),
      '%label' => $label,
      '%guid' => $guid,
      '@errors' => \Drupal::service('renderer')->renderRoot($element),
      ':url' => $this->url('entity.feeds_feed_type.mapping', ['feeds_feed_type' => $this->feedType->id()]),
    ];
    if ($label || $label === '0') {

error: patch failed: src/Feeds/Processor/EntityProcessorBase.php:587
error: src/Feeds/Processor/EntityProcessorBase.php: patch does not apply
Checking patch tests/src/Kernel/Feeds/Processor/EntityProcessorBaseTest.php...

#80 failed because #80 patch with 8.x-3.x branch is up to date with 'origin/8.x-3.x'.
so please closed or fixed this issue.
otherwise some developer spent time to re-rolled patch.

try to fixing some coding standard

Hi the patch #101 applied cleanly. However there are still many issues reported by phpcs. Working on and providing updated patch.

Thanks!

Hello, after updating to the latest version of this module (beta4), which includes this MR originated from this issue I began to have this error:

ArgumentCountError: Too few arguments to function Drupal\feeds\Feeds\Target\File::__construct(), 9 passed in web/modules/contrib/feeds/src/Plugin/Type/Target/MediaTargetBase.php on line 49 and exactly 11 expected in Drupal\feeds\Feeds\Target\File->__construct() (line 101 of modules/contrib/feeds/src/Feeds/Target/File.php).

Therefore I created a patch specifically for this, if you guys can review it, I really appreciate it.

Hi @MegaChriz, thank you for your reply, we were already using that patch, but from comment #56 so it was outdated, I already updated it and applied it, and now the code is already in place. Thanks again!