-
Notifications
You must be signed in to change notification settings - Fork 215
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
[RFC] blocking/serial: make method naming consistent #280
Conversation
r? @eldruin (rust-highfive has picked a reviewer for you, use r? to override) |
For completeness, as I highlighted in #281, different method names ensure unequivocal selection between different traits when they are all pulled in by the prelude.
The method names can certainly be improved, though. Removing the prelude is also an option. |
the blocking/non-blocking delineation makes more sense than the send/write difference to me, i suppose we could also split the prelude for blocking/non-blocking/futures? |
Agreed, but then all blocking methods should have the
That won't work nicely because people end up using both blocking and nb traits in the same mod. Especially since some traits are only available in blocking flavor and others only in nonblocking flavor. Nobody uses the same trait in both blocking and nonblocking flavors at the same time though. IMO naming blocking and nonblocking the same is not that bad. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
ping @ryankurte @therealprof is there anything missing to get this in? also for #282 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, bors r+
bors r- |
Should have a CHANGELOG.md entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks.
bors r+
282: Remove prelude. r=therealprof a=Dirbaio **DEPENDS on #280** Following discussions in #280 #281, we might want to use the same method names for the `blocking` and `nb` (and in the future, `futures`) trait flavors. In that case, importing the prelude guarantees naming conflicts, so it's better to remove it. An alternative would be splitting the prelude in `blocking` and `nb`. However, users often use blocking trait A and nonblocking trait B at the same time, so they'd end up importing both preludes and having name conflicts anyway. Co-authored-by: Dario Nieuwenhuis <[email protected]>
blocking::serial::Write
's methods are namedbwrite_all
andbflush
.b
prefix indicates blocking (I guess?). However no other blocking trait method has this prefix._all
suffix indicates it writes a slice instead of a single word (I guess?). However, no other trait method that writes a slice has this suffix.To make them consistent, this PR renames them to
write
andflush
.