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 log backend in order to deprecate the rtt-log crate #56

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Conversation

t-moe
Copy link
Contributor

@t-moe t-moe commented Nov 30, 2024

@t-moe t-moe requested a review from bugadani November 30, 2024 13:06
Copy link
Contributor

@bugadani bugadani left a comment

Choose a reason for hiding this comment

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

I don't think I can approve this PR, but it looks good to me. I guess I can.

Though I think we should make it clear that these implementations don't prevent third party backends that may be more suitable for particular systems - e.g. backends that don't use global critical sections to better suit multi-core MCUs.

@t-moe
Copy link
Contributor Author

t-moe commented Nov 30, 2024

Ok. I've added a note, regarding possible other options:

+ **Note**: For your platform, particularly if you're using a multi-core MCU, external logger implementations might be better suited than the one provided by this crate via the `log`/`defmt` feature.

@probe-rs/core @sourcebox
Shall we merge & release this, then add a deprecation notice to the rtt-log crate? 👍 👎

@sourcebox
Copy link

@probe-rs/core @sourcebox
Shall we merge & release this, then add a deprecation notice to the rtt-log crate? 👍 👎

I think this can be merged and released. I will then add a note to the rtt-log readme.

@t-moe t-moe added this pull request to the merge queue Dec 2, 2024
Merged via the queue into master with commit ab0fb98 Dec 2, 2024
@t-moe t-moe deleted the feature/log branch December 2, 2024 16:40
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