-
Couldn't load subscription status.
- Fork 79
refactor(macro)!: change to builder pattern #372
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
Conversation
c00a928 to
8bc33fb
Compare
|
Decided to go with builder pattern and move the outer macro back in priority. Would still like to have it, but there are some cases that really need a builder, so this is the foundation. |
4253a65 to
f46ca0a
Compare
|
So this is a chonker... Most of it is taken from #174 which already had some review. I tried to rebase onto current master, but it was to old to get that done cleanly. I pulled what I could and integrated it with the new stuff and added stubs back in. |
migrate outdated builder to current version BREAKING CHANGE: The old macros were dependent on execution order and have been causing trouble with language servers. They are replaced by a builder. See the migration guide at https://davidcole1340.github.io/ext-php-rs/migration-guides/v0.14.html for information on how to migrate. Fixes: #99, #131, #327 Refs: #174, #335 Co-authored-by: David Cole <david.cole1340@gmail.com> Co-authored-by: Pierre Tondereau <ptondereau@users.noreply.github.com> Co-authored-by: Daniil Gentili <daniil@daniil.it>
f46ca0a to
aeed538
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make some comment about API and doc, like i have said in the original PR i think this is the best for the future.
Did not look at the implementation, not that much time, but IMO if the API is good implementation can always be fixed later
Fixes: #337 Co-authored-by: Yoram <yoramdelangen@gmail.com>
aeed538 to
6db05a4
Compare
91a1ae8 to
8060a79
Compare
Other solutions require unstable features
8060a79 to
b6724f0
Compare
|
I'll just merge this now. Has been open long enough for comments. Lets just wait a bit longer before the next release to find issues. |
| .parse_meta() | ||
| .map_err(|_| anyhow!("Unable to parse attribute."))?; | ||
| fn get_method_props<'a>(self) -> ::std::collections::HashMap<&'static str, ::ext_php_rs::props::Property<'a, #path>> { | ||
| todo!() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this was never fixed, so actually getters / setters don't work at all right now
BREAKING CHANGE: The old macros were dependent on execution order and have been causing trouble with language servers. They are replaced by a builder. See the migration guide at https://davidcole1340.github.io/ext-php-rs/migration-guides/v0.14.html for information on how to migrate.
Fixes: #99, #131, #327
Refs: #174, #335