Skip to content

Conversation

Norbytus
Copy link
Contributor

@Norbytus Norbytus commented Apr 6, 2025

I see how you @Xenira changed renaming, maybe change it to trait for common rename things and method?

@Norbytus Norbytus force-pushed the refac/rewrite_rename branch from bf4a705 to 74a88e5 Compare April 7, 2025 16:10
@Xenira
Copy link
Collaborator

Xenira commented Apr 8, 2025

@Norbytus thank you for this contribution. I like this approach.

@Norbytus Norbytus force-pushed the refac/rewrite_rename branch from 74a88e5 to 5c1bc2a Compare April 8, 2025 17:24
@Norbytus
Copy link
Contributor Author

Norbytus commented Apr 8, 2025

@Xenira Also i fix magic method name,
__to_string => __toString
and __call_static => __callStatic

https://www.php.net/manual/en/language.oop5.magic.php

I hope it not make any bc for old version

@Xenira
Copy link
Collaborator

Xenira commented Apr 9, 2025

If I see this correctly this breaks the functionality. The idea was to rename __to_string to __toString

@Norbytus
Copy link
Contributor Author

Norbytus commented Apr 9, 2025

If I see this correctly this breaks the functionality. The idea was to rename __to_string to __toString

Ok, I will revert this changes

@Norbytus Norbytus force-pushed the refac/rewrite_rename branch 3 times, most recently from a244b48 to 1853406 Compare April 14, 2025 05:52
Xenira added a commit to Norbytus/ext-php-rs that referenced this pull request Apr 15, 2025
@Xenira
Copy link
Collaborator

Xenira commented Apr 15, 2025

@Norbytus I added the explicit test cases back in, as I find those are usually better. Seems like they found a problem as well.

Do you want to have a look at it, or should I fix them.

@Norbytus
Copy link
Contributor Author

@Norbytus I added the explicit test cases back in, as I find those are usually better. Seems like they found a problem as well.

Do you want to have a look at it, or should I fix them.

Ohhh, i will fix it. Thanks. I really miss there is mapping from __snake to __camel

@Norbytus Norbytus force-pushed the refac/rewrite_rename branch 3 times, most recently from dbbd58d to 391cda9 Compare April 16, 2025 19:28
@Norbytus Norbytus force-pushed the refac/rewrite_rename branch from 391cda9 to 5353153 Compare April 16, 2025 19:40
@Norbytus Norbytus force-pushed the refac/rewrite_rename branch from 2cd7738 to a9dbedf Compare April 16, 2025 20:06
Copy link
Collaborator

@Xenira Xenira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all your effort. Looks good to me :)

@Xenira Xenira merged commit 201990e into davidcole1340:master Apr 16, 2025
35 checks passed
@Norbytus
Copy link
Contributor Author

Thanks for all your effort. Looks good to me :)

Cool, i think im done. But i make two issue while write test)
I check theme later

@davidcole1340 davidcole1340 mentioned this pull request Apr 14, 2025
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

Successfully merging this pull request may close these issues.

2 participants