Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release major version 5 #588

Open
phil-davis opened this issue Aug 29, 2022 · 4 comments
Open

Release major version 5 #588

phil-davis opened this issue Aug 29, 2022 · 4 comments
Assignees

Comments

@phil-davis
Copy link
Contributor

which will support PHP 7.4 8.0 8.1 8.2 and have type declarations on parameters and return values etc as far as possible with features available in PHP 7.4.

If possible, increase the phpstan level to 6. That requires adding a lot of parameters and return type detail in PHPdoc (specially of the shape of arrays. When I do this locally, I get a longggg list of things reported. So it might take a while to sort all that out! I will try adjusting a few things and then report how it goes, and how big a job it seems.

@mstilkerich
Copy link
Contributor

Hello, was this accidentally published as 4.5.2? 4.5.2 includes huge changes to 4.5.1 including those mentioned in the description of this issue. It also breaks static analysis as the type declarations on many methods have been changed.

Btw, all the @property declarations are wrong. @property must include the $ with the property name, e.g. instead of
@property VObject\Property\FlatText FN
it should be
@property VObject\Property\FlatText $FN

See
https://docs.phpdoc.org/latest/guide/references/phpdoc/tags/property.html
https://phpstan.org/writing-php-code/phpdocs-basics#magic-properties
https://psalm.dev/docs/annotating_code/supported_annotations/#psalm-seal-properties

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 22, 2023

@mstilkerich thanks for pointing this out!

I messed up - I should have cherry-picked only the relevant PR onto the 4.5 branch and released from there. I will do that now, and release a 4.5.3

That should avoid people using the not-yet-ready code that was accidentally released as 4.5.2

See PR #607 - I need to think about the return types that I had to add to some functions. That might be a breaking change (but less of a breaking change than all the stuff that I accidentally released from master as 4.5.2!) So maybe I need to make a 4.5.3 release that really just looks like 4.5.1 and does not do the sabre/xml dependency bump.

@phil-davis
Copy link
Contributor Author

I have released a 4.5.3 from the 4.5 branch. https://github.com/sabre-io/vobject/releases/tag/4.5.3
See my comment there. I think that this should correct the situation, leaving only a couple of very minor added return-type declarations.

Please comment if 4.5.3 causes trouble.

@mstilkerich
Copy link
Contributor

Thanks, yep on my side 4.5.3 is good again. I really only had issues with the psalm type analysis, mostly caused by the change on the magic __get() method of Component that used to have a type annotation of returning a ?Property, but now can also return a Component. And component lacks a toString() method, which will cause type-analysis issues in simple uses like $vcard->FN in a context where a string is needed, which is one of the beauties of this lib.

The issue can partly be remedied with @property annotations, but since these annotations cannot exhaustively cover all the possible VCard/VCalendar properties (plus the technical issue, that properties with a hyphen in their name cannot be annotated by psalm/phpstan for all I know) it will create trouble for static type analysis and require many type annotations or type checks in the code, for what just works fine with v4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants