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

issues-280 Rewrite drivers using proc-macro #292

Merged
merged 11 commits into from
Apr 5, 2022

Conversation

ikrivosheev
Copy link
Member

@ikrivosheev ikrivosheev commented Apr 1, 2022

PR Info

Fixes

Rewrite drivers for: rusqlite, sqlx_* from macro_rules to proc-macro. In proc-macro we can use cfg instead of blank method for types, which enable by crate features and fix: #280

I add cfg for implementation methods which enable by feature like: with-*.

Breaking Changes

  1. Remove methods Value::is_json and Value::as_ref_json when feature: with-json is disabled
  2. Remove methods Value::is_time_* and Value::as_ref_time_* when feature: with-time is disabled
  3. Remove methods Value::is_chrono_* and Value::as_ref_chrono* when feature: with-chrono is disabled
  4. Remove methods Value::is_decimal, Value::as_ref_decimal and Value::decimal_to_f64 when feature: with-rust_decimal is disabled
  5. Remove methods Value::is_big_decimal, Value::as_ref_big_decimal and Value::big_decimal_to_f64 when feature: with-bigdecimal is disabled
  6. Remove methods Value::is_uuid and Value::as_ref_uuid when feature: with-uuid is disabled
  7. Remove methods Value::is_array and Value::as_ref_array when feature: postgres-array is disabled

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 3, 2022

Thank you. This looked extremely brilliant!
Does that mean the examples stay the same with no change?
Btw we might want to add it to the workspace? https://github.com/SeaQL/sea-query/blob/master/Cargo.toml#L2
The only nitpick might be.. can we name the crate sea-query-drivers ?

@ikrivosheev
Copy link
Member Author

@tyt2y3 thank you for review. Done, renamed!

Yes, all examples work as before. But, for example, if feature: with-json does not enable, Value does not have methods: is_json, as_json_ref.

@ikrivosheev ikrivosheev force-pushed the fix/issues-280_fix_drivers branch from c9671df to ebd6188 Compare April 3, 2022 09:03
@ikrivosheev
Copy link
Member Author

@tyt2y3 i rebase branch with master.

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 3, 2022

Thank you for the explanation. I think it also obeys the rule of "feature addition"

@ikrivosheev
Copy link
Member Author

@tyt2y3 what should i do?

@tyt2y3
Copy link
Member

tyt2y3 commented Apr 3, 2022

Oh, nothing more has to be done. Great work indeed. I was thinking the scenario where two downstream creates each with a different feature set in their dependencies. And that would work in theory

@ikrivosheev
Copy link
Member Author

ikrivosheev commented Apr 4, 2022

@tyt2y3 I have seen similar approach in other libraries (for example syn, when features add new functions and methods into crate and types. It is very comfortable and works well in rust.

@ikrivosheev
Copy link
Member Author

@billy1624 @tyt2y3 I have corrected the description

@tyt2y3 tyt2y3 merged commit 317cae5 into SeaQL:master Apr 5, 2022
@tyt2y3 tyt2y3 changed the title issues-280 Rewrite drivers to proc-macro issues-280 Rewrite drivers using proc-macro Apr 5, 2022
@ikrivosheev ikrivosheev deleted the fix/issues-280_fix_drivers branch April 5, 2022 15:04
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.

The trait is not implemented for
2 participants