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

Added input pin impl (requires unproven hal trait) #11

Merged
merged 2 commits into from
Oct 25, 2018
Merged

Added input pin impl (requires unproven hal trait) #11

merged 2 commits into from
Oct 25, 2018

Conversation

ryankurte
Copy link
Contributor

@ryankurte ryankurte commented Oct 10, 2018 •
edited
Loading

Do we have opinions on the use of unproven traits in this library?

@ryankurte ryankurte requested a review from a team as a code owner October 10, 2018 05:38
@ryankurte
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Oct 10, 2018
@bors
Try #11:
80bc5c8
@bors
Copy link
Contributor

bors bot commented Oct 10, 2018

try

Build succeeded

@hannobraun hannobraun added the S-waiting-on-review Status: Review is incomplete label Oct 10, 2018
@therealprof
Copy link
Contributor

@ryankurte Well, you could submit a PR to close rust-embedded/embedded-hal#41 . ;)

@ryankurte
Copy link
Contributor Author

@therealprof I considered that but it would seem impolite to land it before @eldruin's rust-embedded/embedded-hal#97

@ryankurte ryankurte mentioned this pull request Oct 11, 2018
Closed
@therealprof
Copy link
Contributor

@ryankurte As mentioned in the PR you just closed, I do see them as two separate topics. Making this unproven does not preclude making it fallible since this will end up in two different (incompatible) HAL versions anyway.

@caemor
Copy link
Contributor

caemor commented Oct 25, 2018

Would it be possible to release and accept this PR as the embedded hal discussion seems to still need a bit more time?

therealprof
therealprof approved these changes Oct 25, 2018
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.

LGTM

@therealprof
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Oct 25, 2018
@bors @ryankurte
Merge #11
9cd6937 
11: Added input pin impl (requires unproven hal trait) r=therealprof a=ryankurte

Do we have opinions on the use of `unproven` traits in this library?

Co-authored-by: Ryan Kurte <ryankurte@gmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 25, 2018

Build succeeded

@bors bors bot merged commit 4513004 into master Oct 25, 2018
@bors bors bot deleted the feature/input-pin branch October 25, 2018 07:34
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

Assignees
No one assigned
Labels
S-waiting-on-review Status: Review is incomplete
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ryankurte @therealprof @caemor @hannobraun