Update our macros to work in the 2018 style #2248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weiznich
reviewed
Dec 20, 2019
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.
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.
pickfire
reviewed
Feb 11, 2020
pickfire
reviewed
Feb 12, 2020
pickfire
reviewed
Feb 12, 2020
|
I would like to merge this PR soon.
|
This was referenced Feb 27, 2020
martell
reviewed
Mar 4, 2020
martell
reviewed
Mar 4, 2020
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 <pickfire@riseup.net>
Co-Authored-By: Ivan Tham <pickfire@riseup.net>
weiznich
force-pushed
the
sg-2018-macros
branch
from
12ab00c
to
edb7353
Compare
weiznich
force-pushed
the
sg-2018-macros
branch
from
ca9683d
to
2c9e39d
Compare
|
🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 dieselat 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 dieselfrom the doctests.Note that this does not change anything outside the main crate. In
particular,
diesel_cliwill still generate aschema.rsthat 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