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

docs: clarify Signal::derive() behavior #3351

Merged
merged 2 commits into from
Dec 16, 2024
Merged

docs: clarify Signal::derive() behavior #3351

merged 2 commits into from
Dec 16, 2024

Conversation

gbj
Copy link
Collaborator

@gbj gbj commented Dec 11, 2024

@benwis No rush -- Does this seem like a reasonable way to add context to Signal::derive()?

Copy link
Contributor

@benwis benwis left a comment

Choose a reason for hiding this comment

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

I like it, a lot clearer verbiage

@tqwewe
Copy link
Contributor

tqwewe commented Dec 12, 2024

Very clear! Definitely an improvement that would clear the initial confusion I had.

The (and convert it into a Signal<_>) part seems a little redundant but definitely just a nit pick

@bicarlsen
Copy link
Contributor

bicarlsen commented Dec 12, 2024

There's two small typos on line 101

/// Values can be access with the same function call ...

Should be

/// Values can be accessed with the same function calls ...

I've added these as review comments.

@gbj gbj force-pushed the clarify-signal-docs branch from b954fa6 to ef59e11 Compare December 16, 2024 14:15
@gbj gbj merged commit 6b50179 into main Dec 16, 2024
7 checks passed
@gbj gbj deleted the clarify-signal-docs branch December 16, 2024 14:22
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.

4 participants