-
-
Notifications
You must be signed in to change notification settings - Fork 28
Fixes and improvements to meshtastic.rs
generation
#54
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
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.
Thanks for the fix @krant. I can confirm that this fixes the issue, and successive builds with --features ts-config
, --features serde
, and the default features all succeed and leave the project compilable afterwords without issue.
I left one small nonblocking comment.
build.rs
Outdated
#[cfg(feature = "serde")] | ||
{ |
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.
Would we want to take a similar approach for the #[serde(rename_all = \"camelCase\")]
attribute just below here as well?
Also, I think the two #[allow(clippy::*)]
attributes may be something that can be added independent of whether the serde
feature is enabled or not. That's at least what my understanding was here in this PR last week: #52 (comment)
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.
That's a nice improvement, thanks @brannondorsey. build.rs
looks way better now.
meshtastic.rs
generation
Please fix the conflict and then I can merge |
@lukipuki rebased on top of main |
meshtastic.rs
the same forts-gen
andserde
features. Fixes Building with--features ts-gen
corrupts future builds without it #53.meshtastric.rs
instead of per-struct approach.