Nadrieril / dhall-rust

Maintainable configuration files, for Rust users
Other
303 stars 27 forks source link

Remove redundant semicolons #215

Closed cstorey closed 3 years ago

cstorey commented 3 years ago

Hi!

Thanks for your work on dhall!

The release of rust 1.51 brings a new redundant_semicolons lint. Unfortunately, the code that generates type assertions in the procedural macro crate will trip this lint. Because I (and I'm sure other folks) tend to run builds with -Dwarnings enabled, this results in build failures, since the compiler can't tell between a spare semicolon in my code, or from third party macros.

The most useful change is in the procedural macro crate, but I've also removed extra semicolons in the tests for good measure.

I see there's also a bunch of style check failures from capitalised acronyms, but a lot of those seem to form part of the public API, which would make fixing those non-trivial. I'd be tempted to liberally sprinkle #[allow(clippy::upper_case_acronyms)] everywhere for now, but some of them exist in generated code, so I'm not sure what you'd like to do abuot that.

Thanks,

Nadrieril commented 3 years ago

Hi! Thanks for taking the time to fix this. You're changing a lot of stuff, is this all for lints/clippy's sake? Most of it looks fine to me, except intersperse and expr!() that I don't understand. Why those changes?

cstorey commented 3 years ago

Hi,

Those questions are more than fair. It's a good reminder to add pre-emptive comments for the reviewer :)

I've added a quick comment on the expr macro changes.

Does that seem reasonable to you?

Nadrieril commented 3 years ago

Cool that looks good, thanks!

cstorey commented 3 years ago

Thanks!

Nadrieril commented 3 years ago

The fix is in v0.10.1

cstorey commented 3 years ago

That's brilliant, thanks!