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 set_all_channels() for setting on, off counter values and full-on and full-off in one transaction #18

Merged
merged 6 commits into from Apr 5, 2024

Conversation

mhthies
Copy link
Contributor

@mhthies mhthies commented Mar 25, 2024

Follow-up and alternative solution to PR #17. See discussion over there for detailed description of the problem and advantages and disadvantages of the solution approach.

@coveralls
Copy link

coveralls commented Mar 26, 2024

Coverage Status

coverage: 83.103% (-0.8%) from 83.883%
when pulling 29d017f on mhthies:feature/set_all_full_on_off_2
into 9865710 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.

Great, thank you!
Could you also add an entry to the changelog?

src/channels.rs Outdated Show resolved Hide resolved
tests/channels.rs Outdated Show resolved Hide resolved
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.

Sorry, could you also briefly explain this function in the readme/lib.rs like the rest?

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.

Could you run cargo fmt again?

@mhthies mhthies force-pushed the feature/set_all_full_on_off_2 branch from 76f0695 to 29d017f Compare April 5, 2024 20:27
@mhthies
Copy link
Contributor Author

mhthies commented Apr 5, 2024

Done. Sorry for not checking this. I ran cargo doc, but I didn't consider that cargo fmt might dislike a random extra space in the comment.

@eldruin eldruin merged commit 8d85129 into eldruin:master Apr 5, 2024
21 checks passed
@eldruin
Copy link
Owner

eldruin commented Apr 5, 2024

I have published this in version 1.0. Thanks!

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.

None yet

3 participants