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

Signal ergo minor fixes #986

Merged
merged 2 commits into from
Oct 23, 2024

Conversation

SebKuzminsky
Copy link
Contributor

This fixes a couple of docs typos and derives Debug for Signal, SignalReader, and SignalWriter (and CriticalSectionWakerRegistration).

Having Debug available enables e.g. my_task::spawn(my_signal_reader).unwrap();

Copy link
Collaborator

@korken89 korken89 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 for the PR!

Deriving Debug does however not look like the correct thing to do as the underlying datastructures are not made for being printable.
Can you change to implement Debug manually on SignalReader/SignalWriter and just have it print something along the lines of SignalWriter { .. }?

@SebKuzminsky
Copy link
Contributor Author

Ok, will do!

@korken89 korken89 added the skip-changelog Sometimes changes are not significant enough for a changelog entry label Oct 23, 2024
@SebKuzminsky SebKuzminsky force-pushed the signal-ergo-minor-fixes branch from aedab12 to 18ef898 Compare October 23, 2024 18:40
@SebKuzminsky
Copy link
Contributor Author

I deviated from your suggestion a little bit:

my_signal_reader: SignalReader<u16>{ .. }

I'm happy to take the type out if you prefer, lmk.

@korken89
Copy link
Collaborator

That's fine, can you have a look at the failing test? I think it's just a cargo fmt missing.

This facilitates e.g. `my_task::spawn(my_signal_reader).unwrap();`
@SebKuzminsky SebKuzminsky force-pushed the signal-ergo-minor-fixes branch from 18ef898 to 7773fc9 Compare October 23, 2024 18:49
@SebKuzminsky
Copy link
Contributor Author

That fixed it, only took 3 tries 🙄
I see some new stuff went into master, do you want me to rebase?
Thanks for your patience and guidance...

@korken89 korken89 added this pull request to the merge queue Oct 23, 2024
@korken89
Copy link
Collaborator

Thank you for the improvement, and no worries!

Merged via the queue into rtic-rs:master with commit 1461977 Oct 23, 2024
55 checks passed
@SebKuzminsky SebKuzminsky deleted the signal-ergo-minor-fixes branch October 23, 2024 19:00
@AdinAck
Copy link
Contributor

AdinAck commented Oct 29, 2024

Haha, fascilitates, facilitates, tomato tomAto! 😅

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Sometimes changes are not significant enough for a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants