Revert changing method signatures

[ysfks]

This PR reverts changing method signatures in #607 and was tasked by @kohlerdominik

After merging this PR, the v6-branch should be in state to allow a new patch-release, as the breaking changes were reverted.

[jonnott]

@julesgraus .. are you cool with this?

[julesgraus]

Yup

[julesgraus]

Should I apply my changes on a v7 branch if they are reverted for v6? Or how would you want to proceed?

[jonnott]

So @julesgraus @ysfks .. do you think I'm good to tag 6.1.0 ? or 6.0.6 ? (votes please!)

I think dropping PHP versions is maybe good reason to go to 6.1.0 (or does that not count as a 'feature' ? :) )

[jonnott]

Should I apply my changes on a v7 branch if they are reverted for v6? Or how would you want to proceed?

Let's get this next version out first, then see where we are. I feel like a new major version should be a @lazychaser thing.. ?

[jonnott]

Also, should we be testing against PHP 8.3 and 8.4 ? @ysfks @julesgraus

[ysfks]

@jonnott I'm in favor of 6.0.6 as this is a fix for 8.4 warnings not a feature. But it would be good to test it against 8.4 as the title points out. Thx for merging, btw.

[jonnott]

@jonnott I'm in favor of 6.0.6 as this is a fix for 8.4 warnings not a feature. But it would be good to test it against 8.4 as the title points out. Thx for merging, btw.

But what about the fact we're dropping support for some PHP versions too?

[kohlerdominik]

So @julesgraus @ysfks .. do you think I'm good to tag 6.1.0 ? or 6.0.6 ? (votes please!)

SemVer doesn't care about the environment, only about the codes functionality, so IMO you should look at that isolated:

• Does the release include code changes that breaks the API (namingly parameter and return types): No = no Major release
• Does the release include code changes that keeps exising API intact, but releases new features: No = no Minor release
• Does the release include code changes that keeps existing API intact and fixes issues on the code: Yes = Patch release

So 6.0.6 is the correct release version.

I think dropping PHP versions is maybe good reason to go to 6.1.0 (or does that not count as a 'feature' ? :) )

If we would talk about adding support for a Major PHP-version, we could argue creating a Minor or even Major release "feels" right.

However, it's never needed as explained by the doctrine-project, as the package manager will resolve this correctly, therefore apps with an environment < 8.0 will simply not get the new release.

Also, should we be testing against PHP 8.3 and 8.4?

composer.json defines "php": "^8.0", as requirement, which means I would suggest to define the test-matrix as: php: [8.0, 8.1, 8.2, 8.3, 8.4]

[jonnott]

composer.json defines "php": "^8.0", as requirement, which means I would suggest to define the test-matrix as: php: [8.0, 8.1, 8.2, 8.3, 8.4]

The tests fail for PHP 8.0 (composer is unable to install a compatible set of dependencies), so should probably bump this to ^8.1 for the 6.0.6 release do you think @kohlerdominik ?

[kohlerdominik]

Looks like for some reason I can't understand laravel/serializable-closure was added to the the requirements. But everything seems to work without it aswell, see #623.

[jonnott]

@ysfks Looking at v6.0.5...v6 it doesn't appear to me that every single method signature change has been reverted. Are you happy that these few methods appearing in that diff are ok i.e. non-breaking changes?

[ysfks]

@jonnott I checked it and couldn't see anything related to that. Can you show me which ones are left untouched/non-reverted? I see only two methods affected in the logs and they are necessary changes for 8.4

[jonnott]

@jonnott I checked it and couldn't see anything related to that. Can you show me which ones are left untouched/non-reverted? I see only two methods affected in the logs and they are necessary changes for 8.4

@ysfks As long as you are happy that these method changes can go into the next v6.0.6 release without causing any issues with PHP versions (i.e. deprecation warnings) or BC breaks, then we're all good..