v4 release with breaking changes

[Philipp91]

I'm bundling a few changes that we could roll into a v4 release that's (mildly) breaking for clients. Perhaps @nemiah could create a branch for it, as I wouldn't want to send a PR against master for this.

As for the "camel case for acronyms" discussion from #496, I consulted the Symfony style guide we use (https://symfony.com/doc/8.0/contributing/code/standards.html#naming-conventions), which has nothing to say on the matter. Personally, I'm much more comfortable with properly camel-cased acronyms (SendSepaTransfer) than the alternative (SendSEPATransfer), even though the latter is currently the slight majority in phpFinTS. But then there's this recent RFC for the PHP standard library itself (https://wiki.php.net/rfc/class-naming-acronyms) which very strictly requires proper camel case and which was almost unanimously accepted. So I'd say we should go that route too. Unless someone objects, I'll do a bulk rename and add that to v4. This would exclude segment names like HKTAN and the MT family of standards like MT940. And of course the brand name of the library can continue to be phpFinTS and not phpFinTs.

[Philipp91]

Another thing to include here could be the migration from phpDoc @Foo annotations to native PHP attributes. Those should be much more efficient to parse than the regex-based extraction we're using today. That could save some CPU because this process runs from scratch in each PHP process, for all of the segments used by the bank's BPD/UPD or by any of the actions the user executes.

Although we couldn't get rid of the phpDoc parser entirely, because native PHP typed properties don't support sth like string[] yet.

[Philipp91]

I'm including the cleanup of the serialization migration: #364

[nemiah]

I see no problem sending breaking changes to master. The releases are distributed through composer and not the master branch. In my opinion, this should only affect someone pulling directly from master and they should know better 😁

[Philipp91]

I can imagine people using dev-master (a) regularly because we don't release as regularly as they like, or (b) concretely now because they need VoP and want to test our draft implementation, which will land before breaking changes for v4.

Also, having v4 on a separate branch would allow us to keep developing the VoP feature (which those users who use FinTS to send transfers will desperately need) on the master branch and eventually release it without forcing a breaking change on users who want VoP.

[leobeal]

@nemiah The last release was in March, and there has been important fixes since then, for example 43d321c.

I am one of those using dev-master.

[leobeal]

I would suggest naming changes that are not breaking (i.e. private methods naming) are iteratively merged into master, so the v4 changes does not become a huge PR hard to review. Unfortunately the code coverage is very low.

[nemiah]

So, I'll make a new release and we push the VoP changes to master, because we're not going to maintain a version with VoP and a version without VoP, are we?

[Philipp91]

Sorry for the misunderstanding. Maybe it's easiest if you just forget v4 for now.

We'll finish implementing VoP and we'll ship it as 3.x, so that people can pick it up as a non-breaking change. Whatever is breaking I'll pile up on a branch on my end, and once we're done with VoP I'll start sending that towards master in small PRs; at the end of that process we can release v4.

So right now, we need neither a release nor a new branch in the main repo, we'll just work on VoP.

[nemiah]

I have created a new release, because this version seems to be well tested and now we can have fun 😊

[Philipp91]

Wenn wir Release Notes zu VOP und/oder zu Breaking Changes in v4 machen, sollten wir erwähnen, dass man für SEPA-Transfers am Ende $action->ensureDone(); aufrufen sollte.

[leobeal]

Hi,
Is there any reason min php version was bumped to 8.3 from 8.0?

It seems to be all tests were passing on 8.0 - are not failing simply due to composer requirement constraints. If we don't have a reason to bump the requirement, I'd suggest we rollback to 8.0

[nemiah]

Yeah, it was a spontaneous idea, I fixed it.