Skip to content

Improve safety and test coverage #14

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

Merged
merged 3 commits into from
May 23, 2025
Merged

Conversation

Qard
Copy link
Member

@Qard Qard commented May 23, 2025

This eliminates all unwrap() and expect() uses, replacing them with proper error propagation, so there should no longer be any possibility of crashing, it should just throw errors into JS where relevant.

I've also updated the doc tests to be correct and ensured they are run in CI properly.

Additionally I did some restructuring to split up the code in the php crate a bit so it's not just in one giant file. 😅

It also enables clippy linting, which was disabled for a while as there was a lot of noise from things which needed adjusting.

@Qard Qard force-pushed the safety-and-test-improvements branch 8 times, most recently from 1322fab to e8c8d80 Compare May 23, 2025 12:55
@Qard Qard force-pushed the safety-and-test-improvements branch from e8c8d80 to a3de659 Compare May 23, 2025 13:17
Qard added 2 commits May 23, 2025 22:35
Because the Linux build happens in a docker task, the PHP headers
are not available to build ext-php-rs, which is required to do the
cargo test.
@Qard Qard force-pushed the safety-and-test-improvements branch from 4702935 to 0b14d3c Compare May 23, 2025 14:50
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 3a70490 into main May 23, 2025
11 checks passed
@mcollina mcollina deleted the safety-and-test-improvements branch May 23, 2025 15:19
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.

3 participants