We currently have two @codingStandardsIgnoreFile directives in our code. One for the auto-generated \Drupal\search_api\ProxyClass\ParamConverter\SearchApiConverter class and one for \Drupal\search_api\UnsavedIndexConfiguration.
I think we should remove both of them, but definitely the latter. That one was added in #2671288-51: Follow drupal coding standards, probably because the code was unused at the time so it didn't seem worthwhile to adapt it to coding standards. Since that doesn't apply anymore, we should remove the directive again and fix any resulting code style issues.
We should be able to do the same for SearchApiConverter by just rephrasing or reformatting some of the text.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff-2914478.txt | 2.6 KB | borisson_ |
| #8 | 2855254-8--dont_ignore_coding_standards.patch | 1.02 KB | drunken monkey |
Comments
Comment #2
borisson_Since
SearchApiConverteris autogenerated, I don't think we should fix it. Core has a couple of similar files as well; see core/modules/node/src/ProxyClass/ParamConverter/NodePreviewConverter.phpComment #3
drunken monkeyWhy? I understand not changing auto-generated files if they will be re-generated at some point, or if the generating code also has to read them again, but none of that seems to apply. To me, it seems there's no real argument here against fixing coding standards in that file.
(And especially regarding coding standards, "Core also does it" is no valid argument.)
I'm not hell-bent on doing it, though, I just don't see a reason why not.
Comment #4
borisson_I don't see a reason why we should do it. Generated files are not something we should worry about.
Comment #5
borisson_Actually, both files are autogenerated. I wanted to start working on this but I feel like we can all use our time better :) Let's keep those 2 files as-is, close this issue and move on.
Comment #6
drunken monkeyOf course we can use our time better, especially as long as there's no stable release. But if that's a reason to close an issue, 99% of issues on d.o could just be closed. What harm is there in keeping it open, and seeing if maybe some "novice" comes along who wants to work on this?
Comment #7
dev.patrick commentedAdding patch to remove @codingStandardsIgnoreFile at \Drupal\search_api\ProxyClass\ParamConverter\SearchApiConverter
Comment #8
drunken monkeyThanks!
However, PHPCS then complained (at least for me locally – the test bot doesn't seem to use that check) that namespaced classes aren't supposed to have a file comment, so I moved that to the class doc comment instead.
Comment #9
borisson_Applied the patch and ran phpcs, no warnings.
Comment #11
drunken monkeyI don't think the attachment was intended? ;)
Anyways, thanks for reviewing!
Committed. Thanks again, everyone!
Comment #12
borisson_No, looks like that was from another issue.