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.

Sign up for GitHub

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

Jump to bottom

[Reorganization Proposal 1] Move traits into blocking/nonblocking modules #278

Merged
merged 1 commit into from
May 19, 2021
Merged

[Reorganization Proposal 1] Move traits into blocking/nonblocking modules #278

merged 1 commit into from
May 19, 2021

Conversation

eldruin
Copy link
Member

@eldruin eldruin commented May 7, 2021 •
edited
Loading

As discussed in our weekly meeting.
I chose nonblocking as module name because nb could be misunderstood as a re-exported nb crate.
An alternative is #279

@eldruin eldruin requested a review from a team as a code owner May 7, 2021 07:00
@rust-highfive
Copy link

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Review is incomplete T-hal labels May 7, 2021
burrbull
burrbull previously approved these changes May 7, 2021
View reviewed changes
@eldruin eldruin mentioned this pull request May 7, 2021
Closed
@ryankurte
Copy link
Contributor

is it worth considering this in the context of #172 (and the possible provision of future-based non-blocking types?)

@eldruin
Copy link
Member Author

eldruin commented May 11, 2021

As we discussed in the meeting, async traits would live in a parallel asynchronous/futures/... module besides blocking and nonblocking/nb, since it seems it will not be possible to use nb (the crate) as intended in its version 0.1 to handle all execution models.

@eldruin
Copy link
Member Author

eldruin commented May 14, 2021

Any objections or can we merge this @rust-embedded/hal ?
This PR is a bit of a gatekeeper for any other changes because of the file shuffling.

@eldruin eldruin mentioned this pull request May 14, 2021
Closed
11 tasks
@ryankurte
Copy link
Contributor

parallel futures module seems reasonable and i don't have strong opinions for / against, lgtm

@eldruin
Copy link
Member Author

eldruin commented May 19, 2021

Alright, can I get an actual approval here @rust-embedded/hal?

therealprof
therealprof approved these changes May 19, 2021
View reviewed changes
Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Seems we're all in agreement here, so let's do this.

bors r+

@bors
Copy link
Contributor

bors bot commented May 19, 2021

Build succeeded:

@bors bors bot merged commit b60c39a into rust-embedded:master May 19, 2021
@eldruin eldruin deleted the reorganize-blocking-nb branch May 19, 2021 09:13
@GrantM11235 GrantM11235 mentioned this pull request Aug 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@therealprof therealprof therealprof approved these changes

@burrbull burrbull burrbull left review comments

Assignees

@therealprof therealprof

Labels
S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eldruin @rust-highfive @ryankurte @burrbull @therealprof