Problem/Motivation

The testing configuration is out-of-date.

Proposed resolution

Update testing configuration to match the current template and make coding standards fixes.

Remaining tasks

Implement.

User interface changes

None.

API changes

None.

Data model changes

None.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

liam morland created an issue. See original summary.

liam morland’s picture

Assigned: liam morland » Unassigned
Status: Active » Needs review
berdir’s picture

Status: Needs review » Needs work

this likely needs to be split up in more steps, we should also either in those issues or here mark the validation jobs that are fixed as allow failure false.

to void conflicts between diffrent phpstan jobs, phpstan on previous major should be disabled.

liam morland’s picture

I have switched it so that this issue is only about updating the testing configuration. Things like phpcs fixes can be done in follow-up issues.

liam morland’s picture

Status: Needs work » Needs review
liam morland’s picture

Doing all the phpcs issues one at a time will take a long time. Would you accept a merge request that fixes the issues that can be fixed automatically by phpcbf?

berdir’s picture

Depends on the result. phpcbf includes several fixes that I think are pointless, such as adding empty/placeholder docblocks just to get rid of missing docblock complaints, I will not accept that, that's not an improvement. comment stuff in general can be challenging to review.

I'm not requesting to have a separate issue for every single distinct issue.

A compromise might be an issue that does phpcbf but excludes anything that changes comments, at least wording part. Although something like adding a . to a weird non-sentence comment isn't really going to fix it, so I think an easy rule is just ignore anything comment related in a first pass.

berdir’s picture

another challenging topic with phpcbf is changing types in functions which technically is a BC break, such as adding a arrays to variables documented as array. That's kind of still allowed as this is still in beta, but I also don't think it's worth possibly breaking someone's code over that.

liam morland’s picture

The advantage see of making a commit with phpcbf fixes is that it would fix a lot of issues with not a lot of work. Even an empty docblock means there is a clear place to add comments. It can serve as a reminder to someone working on a function that it ought to be documented. Such a person would be in a good position to document because they are already familiar with the function.

liam morland’s picture

I suggest having a follow-up for phpcs fixes so that this can be committed.