Skip to content
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 /comments related errors and their integration tests #452

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

oguzkocer
Copy link
Contributor

Follows established patterns to implement error types for /comments and their integration tests. It also introduces CommentCreateParamsBuilder type to make it easier to build a CommentCreateParams type in Rust.

There is some discussion and pre-RFCs for using something like the ..Default::default() syntax for all the remaining fields when the type itself doesn't implement Default, but the remaining fields do. If such a feature is ever implemented, we could drop these builders.

There are also proc macro crates that implement these, but the ones I tried before didn't work out the way I'd like. For example, most build functions will return a Result<T, E>, but that only makes sense when the build might fail. In this builder type, I make the mandatory fields for the params type also mandatory for the builder, so we don't require any error handling.

In any case, I think this setup will serve us well on the Rust side for now. In native wrappers, this problem doesn't exist because we can use the #[uniffi(default = None)] macro attribute and both Kotlin and Swift can take arbitrary number of arguments in their constructor.

@oguzkocer oguzkocer merged commit 5df7bbb into trunk Dec 19, 2024
21 of 23 checks passed
@oguzkocer oguzkocer deleted the comments-errors branch December 19, 2024 09:27
@crazytonyli
Copy link
Contributor

@oguzkocer This PR's integration tests failed, and it caused the trunk branch and a few other PRs to fail.

@oguzkocer
Copy link
Contributor Author

@crazytonyli Thank you for letting me know. I added the buildkite/wordpress-rs/wordpress-rust-wordpress-6-dot-6 task to the required checks, so that this won't happen again. I'll open a PR fixing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants