Prepare changes to the macro crate.#77
Conversation
gibbz00
force-pushed
the
macro_refactors
branch
from
f5de00f to
b5d664a
Compare
| discard_lifetimes, meta_items, ContainerMeta, FieldMeta, Mode, Namespace, VariantMeta, | ||
| }; | ||
| use crate::meta::{ContainerMeta, FieldMeta, Mode, VariantMeta}; | ||
|
|
There was a problem hiding this comment.
Nit: no empty line between crate and super imports, please.
| de::from_xml(&ast).into() | ||
| } | ||
|
|
||
| pub(crate) use container::{ContainerMeta, Mode}; |
There was a problem hiding this comment.
Not a fan of this change. IMO 100-200 line files are a net negative to my ability to understand the code, my optimal size is around 300-800 lines of non-test code.
There was a problem hiding this comment.
Understand. For me it's a visibility modifier reason to limit types and their impl blocks to their own module, with quite a few exceptions of course. Easier to reason about them then, and what external libraries each part uses. Save thing as with star imports, ok if i flatten things in the end?
There was a problem hiding this comment.
Similarly to the other aspect, I don't want this right now. If you end up adding substantial logic for one of these types we can definitely consider splitting it out at that point.
gibbz00
force-pushed
the
macro_refactors
branch
from
b5d664a to
998a731
Compare
gibbz00
force-pushed
the
macro_refactors
branch
2 times, most recently
from
f5ff553 to
e5ae0dc
Compare
gibbz00
force-pushed
the
macro_refactors
branch
2 times, most recently
from
cfe4675 to
ed7ce1f
Compare
gibbz00
force-pushed
the
macro_refactors
branch
from
ed7ce1f to
f0e210b
Compare
| @@ -0,0 +1,14 @@ | |||
| mod container; | |||
There was a problem hiding this comment.
IMO this is still overindexing on small modules. I think it would make sense to move meta.rs to meta/mod.rs, and only split out namespace into a separate file for now.
|
Recreated the commits for cleaning up useless explicit lifetimes in #80, closing this for lack of followup. |
2 participants
Feedback from djc/instant-epp#56 resulted in the
suggestion of at least 3 new potential macro attributes, (from, into, remote).
Before tackling those, however, I'd like to do some refactors and improvements
to the macro crate. Specifically in terms of making it clearer of when a
attribute belongs to a field, a container; when it is for serialization,
deserialization; and lastly, when it is for named, unnamed, or unit fields of
structs and enums. (There are at least 2 x 2 x 3 = 12 places where attributes
might be valid, not taking into account cases when different attributes are
incompatible with each other.)
Validation is scattered in the codebase, errors are seldom error emitted, and
there's little user documentation to rely on. This PR is the first one in a
series whose goal is to tackle exactly those 3 points. This once splits up some
larger files and uses
use crate::*;to improve locality of behavior.To clarify, this PR changes no behavior whatsoever.