Skip to content
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

Add feature-gated support for defmt::Format #20

Merged
merged 5 commits into from
May 23, 2024

Conversation

ShakenCodes
Copy link
Contributor

Supports defmt::Format operating with the types in this driver. This is especially useful for outputting Errors. To enable operating with defmt, set the feature "defmt" in the dependency.

@coveralls
Copy link

coveralls commented May 16, 2024

Coverage Status

coverage: 88.71% (-0.09%) from 88.8%
when pulling 38a38c0 on Radiator-Labs:master
into 3a222c4 on eldruin:master.

Copy link
Owner

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, thank you!
Could you also:

  • Rename the feature defmt-03
  • Add an entry to the changelog
  • Document this feature in the readme and lib.rs?
    This repo has en example.

@ShakenCodes
Copy link
Contributor Author

Changes have been made. I chose not to add doctest examples in lib.rs, as I do not know how to enable the feature within the doctest context. Since usage is basically like anything else that derives debug or format, this is a pretty trivial usage. Hopefully the example in the readme is sufficient.

Copy link
Owner

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
I would also just add a couple of lines about the defmt-03 feature to the lib.rs, without examples.

CHANGELOG.md Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Comment on lines +117 to +122
To enable defmt support, when specifying a dependency on eeprom24x, add the feature "defmt-03"

```toml
[dependencies]
eeprom24x = { version = "0.7.2", features = ["defmt-03"] }
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would shorten this section to just this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting deleting the two previous paragraphs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still want to add something to lib.rs? I could just move or duplicate the usage example from README.md

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I would remove the introduction and example and just make "defmt" into a link to the crates.io page.
Then we are never outdated. Furthermore, I think people will most probably either already know about defmt or not care much about it.

ShakenCodes and others added 2 commits May 22, 2024 16:22
Co-authored-by: Diego Barrios Romero <eldruin@gmail.com>
Co-authored-by: Diego Barrios Romero <eldruin@gmail.com>
Copy link
Owner

@eldruin eldruin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@eldruin eldruin merged commit a8a740f into eldruin:master May 23, 2024
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants