Remove conflicting implementation of 'Expr'#2604
Conversation
Expurple
left a comment
Expurple
left a comment
There was a problem hiding this comment.
Thank you.
Testing
For testing, one needs to append this to the workspace Cargo.toml:
[patch.crates-io]
sea-query = { git = "https://github.com/Huliiiiii/sea-query.git", branch = "expr-1" }Actually, I'd temporarily include this change in the PR, while it's a draft and while SeaQL/sea-query#889 isn't merged. This would allow running CI checks on your PR. We need that.
My review
Everything seems OK. cargo test --workspace passes, but prints new warnings:
use of deprecated associated function `sea_query::SimpleExpr::expr`: Please use the [`SimpleExpr::new`] methodCould you fix the warnings, please?
I haven't run the other test configurations locally. I'd let the CI do that, after you include [patch.crates-io] in the PR.
|
Regarding the warning, I am not sure whether SimpleExpr::expr should be deprecated yet, I am waiting for a response from the maintainer. |
|
@tyt2y3, can you trigger a CI run, please? Both PRs look ready overall and only need your input on some details |
|
it looks small enough, nice. |
|
Apart from unrelated Now we can proceed with the merge:
|
|
I'll fix it on master |
3 participants
PR Info
ExprandSimpleExpras one type sea-query#889New Features
Bug Fixes
Breaking Changes
Changes