#Coding standards

Pipeline has errors. Would recommend verifying that everything passes before putting into review please.

@quietone Please review.

dhruv.mittal → changed the visibility of the branch 3373530-fix-the-warningserrors to hidden.

@kul.pratap, Thanks for asking! The suggested changes to phpcs.xml.dist should exclude the files that have just started to cause violations from being checked by phpcs. Without those being checked that will bet this back on track to where only the feedback from larowlan and myself need to be resolved.

The files that have just started to cause the violations are going to be fixed in the parent issue, along with enabling the sniff on all of core. I've already set that up and postponed that on this issue.

I hope that all make sense.

Created MR with what should be the remaining fixes.

And postponing on the last child.

Due to recent commits of coding standard fixes, there are too many changes to fix and enable this sniff in 📌 Fix remaining missing @var annotation that do not have a default value RTBC . So, opening this up again to do the final enable and fixes.

@quieton Should I resolve the unresolved issues in the MR as mentioned by you?

Fixed the issues reported by phpcs
Kindly review :)

@kul.pratap, thanks. I just added another change to exclude files in 'Plugin' directories. That should help here.

Updating title for the new scope.

I'll work on an issue for the changes in the 'Plugin' directories.

@quietone Thank you for the feedback. I will review the comments in the MR and address them.

@kul.pratap, Welcome to Drupal! Thanks for working on this. I made a few comments in the MR. Also, This is failing the pre commit checks. It is good practice to Run core development checks → locally before submitting a change.

There have been recent commits that are causing more errors. I don't think the sniff can be enabled here. This will have to go back to doing a subset of files. I'll look into that now.

kul.pratap → made their first commit to this issue’s fork.

Working on it.

No conflicts with the rebase, restoring RTBC

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Seems pretty straight forward.

I rebased the MR because I couldn't re-run, but the build step had failed which I'm hoping isn't a new random issue popping up. But rebase based now just our regular set of random failures.

It looks like code duplication was made at core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php
There is fix at 📌 Remove FormattableMarkup from FileManagedUnitTestBase Active .

Automatically closed - issue fixed for 2 weeks with no activity.

The changes looks good to me
Moving it to RTBC.

Fixed the phpcs pipeline.
Kindly check :)

I forgot to update the IS. Also adding credit.

📌 Fix 'Drupal.Semantics.FunctionT.NotLiteralString' coding standard Active committed, and removed from the issue summary.

linting check passed so I am setting this to needs review.

One child completed.

Thank you to everyone who helped to complete this issue. The last child was recently committed. So, I am closing this as fixed.

Linting passed, so this can be reviewed.

linting passed, try again.

Committed to 11.x - fixed

larowlan → closed merge request !8393

larowlan → closed merge request !8390

Committed to 11.x - thanks!

larowlan → closed merge request !10436

Committed to 11.x - thanks

I don't know that there's an automated rule we can us for this, but it would be good to add a follow up to investigate

Whack a mole, needs another reroll 😭

Committed to 11.x - thanks!

larowlan → closed merge request !8954

quietone → closed merge request !4216

Couple of minor issues in the MR, also needs a reroll

Thanks for working on this.

Left some comments on the MR
This also needs re-roll - thanks for working on this!

Rebased, and linting checks have passed, restoring RTBC

Rebased, linting has passed, although other tests are still running.

Committed to 11.x - thanks!

larowlan → closed merge request !10165

larowlan → closed merge request !10375

Committed to 11.x - thanks!

larowlan → closed merge request !10439

----------------------------------------------------------------------------------------------------

Running PHPStan on changed files.
 ------ ------------------------------------------------------------------------------------ 
  Line   modules/filter/src/Plugin/Filter/FilterHtml.php                                     
 ------ ------------------------------------------------------------------------------------ 
  270    Method                                                                              
         class@anonymous/modules/filter/src/Plugin/Filter/FilterHtml.php:265::setTextMode()  
         has no return type specified.                                                       
 ------ ------------------------------------------------------------------------------------ 


                                                                                
 [ERROR] Found 1 error                                                          
                                                                                


PHPStan: failed

This failed on commit check - but that's a filter plugin, not a views filter plugin so is out of scope, reverted that changed and it passed

Committed to 11.x - thanks!
More phpcs rules - nice!

Needs a reroll, fine to self RTBC

Needs a reroll, fine to self RTBC

Committed to 11.x - thanks!

Nice to see another rule enabled

larowlan → closed merge request !9687

chhavi.sharma → changed the visibility of the branch 3492335-coding-standard-issues to hidden.

Working on it.

The changes are just for using the short syntax for array and adding a @var line for a property declaration.

IMO, they are so small that they should be fixed when fixing other issues in the code, if those issues are present. Otherwise, the merge request in this issue can be merged, at project maintainers' discretion.

avpaderno → changed the visibility of the branch 3034265-fix-the-errorswarnings to hidden.

The 8.x-1.x branch is no longer supported; any change needs to be done in the 2.x branch.

Hi @kinyein,

It is recommended to use GitLab CI to run automated fixes, I have enabled the pipeline and as can be seen, the phpcs job failed.

Kindly check

Thanks,
Jake

a.aaronjake → made their first commit to this issue’s fork.

@quietone can you please share the script to generate all the files?

Having code that violates code standards so much is bad example.
I opened Drupal\Core\ProxyClass\Batch\BatchStorage and was thinking that some code styles in my IDE broke, I was not expected to have php code in core, that use 4 spaces as indentation.
As mentioned at #6, current patch provide fix some code styles rules.
Let's implement changes and just ignore few code styles rules instead of all rules.

Adding related idea to implement Property promotion for generated class.

Working on it.

cleavinjosh → changed the visibility of the branch 3373061-phpcs-issues to hidden.

Hi @avpaderno,

Please review, check, and advise. Thank you.

Automatically closed - issue fixed for 2 weeks with no activity.

The issue summary needs to link to the PHP_CodeSniffer report done before changing the project files, not after.

Updated the summary please check.

I have resolved phpstan, stylelint,phpcs and some eslint error. there are phpunit and some eslint yet to resolve.
I don't have much knowledge about phpunit tests. I would love to help if someone guide me how to do them.
Thank you

The issue summary must still be updated.

@avpaderno
Made the changes requested,
Please review.

divyansh.gupta → changed the visibility of the branch 3373061-fixes-error-warning-phpcs to hidden.

dhruv.mittal → made their first commit to this issue’s fork.

Working on it.

Patch does no longer apply to latest codebase in 1.0.x

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.

Automatically closed - issue fixed for 2 weeks with no activity.