-
Notifications
You must be signed in to change notification settings - Fork 123
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
Fixed PHP 8.4 deprecations #185
base: 2.0.x
Are you sure you want to change the base?
Conversation
@erusev Is this repo still being maintained? |
Were you able to run the tests? |
I did run the unit tests on PHP 8.4RC1 on my local machine, and they all passed. It's a backward-compatible change and that just makes the nullable parameters explicitly nullable. |
btw, I noticed that the tests defined in "scripts" in composer.json are largely broken. For example, the path to CommonMarkTestStrict.php and CommonMarkTestWeak.php are incorrect (they should be prefixed with "vendor/erusev/parsedown/"), and even then many of the test scenarios are failing. This is not due to the changes in my PR, though - it looks like some general cleanup is needed in this repo to get it healthy again. |
I've pushed an update to fix some of those issues, but likely more are needed. For example, we should probably add PHP 8.4 to CI and allow it to fail (it will until things like Psalm and PHP-CS-Fixer natively support PHP 8.4 without having to use things like |
I'm not sure. It looked like 2.0.x was more up to date than master (eg it had GitHub Actions instead of TravisCI, it separated code into the src directory), which is why I rebased off it for this pull request. |
Hm, ok, but if we're making a release for these changes, shouldn't it be a |
What's v2 for, then? Its most recent release was a few years after 0.8.x. |
It's probably a version of the extensions that was designed to work with Parsedown 2.0. It's something @aidantwoods was working on but it might be abandoned. I'm also not sure why There's a discussion about the same thing in Parsedown at erusev/parsedown#881. |
@aidantwoods can I merge |
It's been a while, but I think parsedown 1.8.x (and extra 0.8) had some risk of breaking extensions due to internal changes to escaping HTML. I think I put these out as prereleases because I wasn't sure what the BC promise should be for not breaking extensions (given they hook into a |
Do you think it still makes sense to have that branch? The latest release, We just merged the |
If we're happy releasing 1.8 as stable I think there's no need to keep the separate branches around |
Can you create a PR that fixes PHP 8.4 deprecations in You can also use the GitHub workflow at https://github.com/erusev/parsedown as reference. |
Fixes #184