-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add AllowDynamicProperties
annotation
#93
Conversation
Hey @GrahamCampbell, thanks for the pull request. Since dynamic properties are deprecated from 8.2, I think the best way forwards is to ensure we aren't using any in this library. The aim of this library (more recently) is to have stronger typing, so dynamic properties that come up over time (new additions to Packagist's API) would cause a deprecation warning for consumers, as they have done for you, and provide us an opportunity to update this library to suit. From some quick testing on 8.2 I see there are a few properties that need to be added, which I've done locally, but I haven't found a package with |
There are actually many such errors. This was just the first one. We should probably skip setting these rather than setting them dynamically, since adding new fields on the server side is not considered a breaking change.
|
This error happens when looking at any package. |
A reasonable suggestion! I can see many of the warnings you've shown locally with the example scripts in this library, but avatarUrl is one I can't see. If you could share the package name that contains that property, that would be lovely. I haven't been able to find any examples that contain it in the Packagist API examples. |
Interesting. It has been there for 7 years. composer/packagist@a627342 |
I've created a patch for the assignment of dynamic properties (treated as a bug now, in the past this was intended behaviour) at #96, and a new issue to add support for the new properties at #97. Thanks for the discussion @GrahamCampbell! |
Maybe fixes #92. I'm not going to have time to confirm right now,.