Conversation
|
Can't figure out why it's failing? |
LPGhatguy
left a comment
LPGhatguy
left a comment
There was a problem hiding this comment.
Looks pretty good! This feature was simpler than I was expecting it to be I think! :D
LPGhatguy
left a comment
LPGhatguy
left a comment
There was a problem hiding this comment.
Mechanically looks pretty good, I have some more comments on organization that should be final!
|
|
||
| if let Some(meta_ignore_instances) = meta.ignore_unknown_instances { | ||
| snapshot.metadata.ignore_unknown_instances = meta_ignore_instances; | ||
| } |
There was a problem hiding this comment.
Can we bump this block to be above where we set children? We're already assigning to the snapshot's metadata and already have a change there.
|
|
||
| #[derive(Debug, Default, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| struct InitMetaJson { |
There was a problem hiding this comment.
I don't think we need the Json part of this struct name.
Since we don't serialize the struct, we might be able to drop the Serialize derive and all of the skip_serializing_if attributes as well.
| })? | ||
| } else { | ||
| InitMetaJson::default() | ||
| }; |
There was a problem hiding this comment.
I think we can fold this code down even further!
Instead of assigning the struct out here and defaulting to InitMetaJson, can we check for the existence of this file later, and then pull properties directly from it onto the snapshot?
Then we can localize the changes to this function to exactly one region, and limit the impact in cases that don't use an init.meta.json file.
|
@LPGhatguy Done in latest commit |
* A minimum viable product for init.meta.json * Properties support * Add ignoreUnknownChildren support * Apply requested changes * Use reflection guiding * Add a script to the test * Change to ignoreUnknownInstances * Apply requested changes
2 participants
Closes #123.
So far only supports
className, want to know if this is what you expected before I work on this more.