Skip to content

Conversation

@dereuromark
Copy link
Member

@dereuromark dereuromark commented Dec 27, 2022

First stab at #368

It will also require test harness to be fixed up for 8.1+

Declaration of PHP_CodeSniffer\Tests\TestSuite::run(?PHPUnit\Framework\TestResult $result = null) must be compatible with PHPUnit\Framework\TestSuite::run(?PHPUnit\Framework\TestResult $result = null): PHPUnit\Framework\TestResult

As well as (follow-ups?):

  • Move tests into tests/ with proper autoload-dev instead of part of autoload

@dereuromark
Copy link
Member Author

I enabled for now the full set of mult-line declarations, just for testing
We can redude this once we are ready for merging if it is too disruptive and only keep 1 of the 3 pairs

For now the pressing question is how to get the tests working on modern versions of dependencies.

@dereuromark
Copy link
Member Author

Anyone any ideas on the blocker of tests getting green again?

@ADmad ADmad mentioned this pull request Apr 9, 2023
@LordSimal
Copy link
Contributor

Whats the state of this PR?

@dereuromark
Copy link
Member Author

This looks like it needs a bit of rework now.
I would still like to see it added

@ADmad
Copy link
Member

ADmad commented Nov 29, 2024

@dereuromark I see you have added a few more rules regarding trailing comma besides the one I added in #398. So if you want to get them into the next minor now would be the time.

BTW PER 2.0 has added rules regarding trailing comma, so it would be good to follow that https://www.php-fig.org/per/coding-style/#26-trailing-commas

phpcs still doesn't seem to have a PER 2.0 rules set but hopefully it will have one soon making things easier for it.

@dereuromark dereuromark changed the base branch from 5.x to 5.next November 30, 2024 03:42
@dereuromark
Copy link
Member Author

dereuromark commented Nov 30, 2024

I rebased and ran a test con cakephp/cakephp:
266 more files changed on the function signatures, not sure if we want to do this too
The benefit would be less merge conflicts in the future.

@dereuromark
Copy link
Member Author

I ran into slevomat/coding-standard#1715 by the way, also happens without my updates, so this is an issue we need to tackle separately..

@ADmad
Copy link
Member

ADmad commented Nov 30, 2024

266 more files changed on the function signatures, not sure if we want to do this too

That's fine, better to stay consistent about the trailing comma along all constructs. It's not a problem as long as it's auto fixable.

I ran into slevomat/coding-standard#1715 by the way

The slevomat guys a usually pretty responsive, so hopefully should be resolved soon.

@dereuromark
Copy link
Member Author

I added PHPStan.
For level 7 there seem to be only a few issues, but we can tackle them also separately afterwards.

@ADmad ADmad requested a review from markstory November 30, 2024 17:08
@dereuromark dereuromark merged commit 47df6ca into 5.next Nov 30, 2024
@dereuromark dereuromark deleted the 5.x-updates branch November 30, 2024 17:44
@dereuromark dereuromark mentioned this pull request Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants