fix derived ItemKey field rename and impl derive(Attributes) #115
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
softprops
commented
Jul 14, 2020
dynomite-derive/src/lib.rs
Outdated
| @@ -115,6 +115,7 @@ fn parse_attrs(all_attrs: &[Attribute]) -> Vec<Attr> { | |||
| /// # Panics | |||
| /// | |||
| /// This proc macro will panic when applied to other types | |||
| //#[proc_macro_error::proc_macro_error] | |||
There was a problem hiding this comment.
note to self: revisit this before merging. it seems the proc_macro_error crate requires this and I didn't catch that last time.
softprops
changed the title
softprops
commented
Jul 14, 2020
| // impl Attribute for Name (these are essentially just a map) | ||
| let attribute = quote!(::dynomite::Attribute); | ||
| let impl_attribute = quote! { | ||
| impl #attribute for #name { |
There was a problem hiding this comment.
we need to derive Attribute since embedded structs within Items need to be viewed as an Attribute. It turns out that makes this impl quite simple since its basically just a map of attributes anyway!
…hich as it turns out helps to cover up the case where Items omit partition_keys and and pass silently in the night. Let's impl Attributes instead of Item ItemKeys
softprops
force-pushed
the
item-key-struct-field-renames
branch
from
958e2b2
to
cc49dd5
Compare
|
generating some compile tests. noting for future reference. |
This was referenced Jul 14, 2020
Closed
|
Wow, this is awesome, looking forward for 0.9 release! |
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.
What did you implement:
addresses issue with renaming partition key fields which has an affect on derived ItemKey struct when it breaks with rust's conventions.
Closes: #108, #107, #79
How did you verify your change:
unit test and compiler warning checks
What (if anything) would need to be called out in the CHANGELOG for the next release:
a number of related things that are open issues were at play. ItemKey's derive impls clone partition and sort keys from Item structs and rewrote them to their deserialized names but in struct field form yielding less than ideal results when renamed to using non-rust naming conventions.
The impl of this derived(Item) who's impl for deriving Item keys will silently omit generating another ItemKey struct if a partition_key is absent. This was to work around an infinite compilation phase loop. An artifact of this is that there's no check in derive Item to ensure Items actually have partition key defined.
The following things where resolved in this change
whew.