-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update our macros to work in the 2018 style #2248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should have a look at the previous PR's that are doing something similar and apply the suggestions from there also here?
diesel/src/lib.rs
Outdated
@@ -292,6 +291,9 @@ pub mod helper_types { | |||
|
|||
pub mod prelude { | |||
//! Re-exports important traits and types. Meant to be glob imported when using Diesel. | |||
#[doc(inline)] | |||
pub use diesel_derives::*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the better strategy here is to put these reexports in those places where the actual traits are defined that are implemented by the corresponding custom derive. If that trait is reexported via prelude, the proc macro will also be reexported.
I would like to merge this PR soon.
|
The only thing nit I could find I added in a comment.
I am not a @diesel-rs/contributors but I guess it makes sense for it to be okay do that given that we are moving to 2.0 - this is the best time for breaking changes. I can't comment on the other 2 questions but I just wanted to bump this with the 2 potential nits I found. Hoping you can merge this soon :) |
This updates our crate to work without `#[macro_use] extern crate diesel` at the crate root. I've audited all uses of `#[macro_export]`. If it was obvious at a glance that inner macros are local, I've added `local_inner_macros`. Otherwise I changed it to use `$crate::`. Most of the non-public macros can probably have the `__diesel_` prefix removed, but I've left it on to keep things reasonable for 2015 edition users and to minimize the diff. We actually have a surprisingly small number of non-derive macros that are public API. It's pretty much limited to our schema macros and operator generation. The schema macros are exported from the prelude, the operator generation macros are not as they had their prefix removed in 2.0 with the expectation they are always invoked with `diesel::`. I've removed all instances of `extern crate diesel` from the doctests. Note that this does not change anything outside the main crate. In particular, `diesel_cli` will still generate a `schema.rs` that assumes `#[macro_use] extern crate diesel;`. It will need to import the prelude at the top of that file. This will be addressed once that crate is moved to the 2018 edition.
We have public macros that aren't exported from the prelude
This change reorganize where and how we reexport our proc macros. Each custom derive is now reexported with the corresponding trait, so that if you import for example `diesel::deserialized::Queryable` you will now import both, the trait and the derive macro. Additionally documentation for each proc macro is added.
Fix doc tests, that are broken due to missing imports. Additionally add some small doc improvements .
Co-Authored-By: Ivan Tham <[email protected]>
Co-Authored-By: Ivan Tham <[email protected]>
12ab00c
to
edb7353
Compare
ca9683d
to
2c9e39d
Compare
🎉 |
Note: This includes #2247. That PR should be merged first. Look at the last commit only for a smaller diff.
This updates our crate to work without
#[macro_use] extern crate diesel
at the crate root. I've audited all uses of#[macro_export]
.If it was obvious at a glance that inner macros are local, I've added
local_inner_macros
. Otherwise I changed it to use$crate::
. Most ofthe non-public macros can probably have the
__diesel_
prefix removed,but I've left it on to keep things reasonable for 2015 edition users and
to minimize the diff.
We actually have a surprisingly small number of non-derive macros that
are public API. It's pretty much limited to our schema macros and
operator generation. The schema macros are exported from the prelude,
the operator generation macros are not as they had their prefix removed
in 2.0 with the expectation they are always invoked with
diesel::
.I've removed all instances of
extern crate diesel
from the doctests.Note that this does not change anything outside the main crate. In
particular,
diesel_cli
will still generate aschema.rs
that assumes#[macro_use] extern crate diesel;
. It will need to import the preludeat the top of that file. This will be addressed once that crate is moved
to the 2018 edition.
Close #1956