softprops / dynomite

⚡🦀 🧨 make your rust types fit DynamoDB and visa versa
https://docs.rs/dynomite/
MIT License
220 stars 53 forks source link

Introduce `#[dynomite(skip_serializing_if = "..")]` attribute #163

Closed Veetaha closed 3 years ago

Veetaha commented 3 years ago

Tested:

What did you implement:

This basically reflects serde::skip_serializing_if attribute. This will be very useful for us to get rid of manual workarounds where we need to store empty binary sets in a db record.

Also had to fix a clippy lint of implementing Into trait instead of From to let CI pass

How did you verify your change:

Ran tests. I've also moved integration tests into a single test binary as per this article

What (if anything) would need to be called out in the CHANGELOG for the next release:

The new feature is specified in changelog in this diff

softprops commented 3 years ago

Happy to merge this when ci is green. Looks like there are still clippy errors

Veetaha commented 3 years ago

@softprops, I've fixed the remaining lints of clippy and rustfmt. They suddenly appeared because we use stable rust toolchain on CI and once the toolchain updates, new clippy and rustfmt lints are added, which suddenly makes our CI red. I suggest we hardcode the version of the toolchain in our CI to make it resistant to rust updates and update the version explicitly. But that's not related to this PR, we can merge it as is.

softprops commented 3 years ago

hardcode the version of the toolchain in our CI to make it resistant to rust updates

That works too. these issues tend to be rare enough that updating pins becomes its own maintenance issue :) Lets try that and see if it fixes the issue the issue so we can move this pull to merge

softprops commented 3 years ago

derp. its green now. merging