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

[RFC] establish a process for adding new traits #39

Merged
merged 2 commits into from
Feb 15, 2018
Merged

Conversation

japaric
Copy link
Member

@japaric japaric commented Feb 14, 2018

This PR establish a process for adding new traits. The details can be found in the diff. This PR is
also proposing using the embedded-hal-driver and embedded-hal-impl keywords on crates.io to make
implementations of embedded-hal and drivers built on top of it more discoverable.

Feel free to suggest changes to these proposals in the comments.

The PR is also adding a list of (WIP) embedded-hal implementations and embedded-hal drivers. If
your crate is not listed there or if you know of a crate that should be there feel free to send a
follow up PR adding it.


Finally, I'd like to use this chance to call for collaborators that will help me with:

  • Merging PRs that update the lists in the README

  • Triaging issues to make sure that they are properly labeled and that they are following the
    established process.

  • "Shepherding" proposals / discussions by preventing them from going off topic and by helping them
    reach a conclusion / consensus, e.g. "Concern $X needs to be addressed before reaching a
    conclusion", "How would this proposal fulfill use case $Y?", etc.

If you'd like to become a collaborator let me know in the comments!

@therealprof
Copy link
Contributor

I'd love to be a collaborator.

README.md Outdated

To leave the unproven state at least *two* implementations of the trait(s) for different platforms
(ideally, the two platforms should be from different vendors) and *one* generic driver built on top
of the trait(s) must be demonstrated. If, instead, reports indicate that the proposed trait(s) can't
Copy link
Contributor

Choose a reason for hiding this comment

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

There might be cases where a trait is purely internal and/or may not have any drivers using it, like timers, ADCs, RNGs... in that case a full example demonstrating the use should suffice, I think.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's a fair observation. Maybe the requirements should be worded less strongly and be treated as more of a guideline.

@thejpster
Copy link
Contributor

LGTM. Happy to help.

Copy link
Member

@hannobraun hannobraun left a 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, aside from a few nit-picks. I'd be happy to help as a collaborator!

README.md Outdated

To leave the unproven state at least *two* implementations of the trait(s) for different platforms
(ideally, the two platforms should be from different vendors) and *one* generic driver built on top
of the trait(s) must be demonstrated. If, instead, reports indicate that the proposed trait(s) can't
Copy link
Member

Choose a reason for hiding this comment

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

I think that's a fair observation. Maybe the requirements should be worded less strongly and be treated as more of a guideline.

README.md Outdated

[`tm4c123x-hal`]: https://github.com/thejpster/tm4c123x-hal

You may be able to find even more implementations by searching for the [`embedded-hal-impl`] keyword
Copy link
Member

Choose a reason for hiding this comment

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

I think this paragraph is important, but it's easy to miss down here. A casual reader might think it's a note about TI. I suggest moving it up, making it the second paragraph under ## Implementations.

README.md Outdated

[`si7021`]: https://github.com/wose/si7021

You may be able to find even more implementations by searching for the [`embedded-hal-driver`]
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph is less easy to miss, since there are no sub-sections, but I'd still move it above the list of links.

@japaric
Copy link
Member Author

japaric commented Feb 15, 2018

Thanks everyone for the feedback. I've incorporated the suggestions. As there's no objection let's land this!

@homunkulus r+

@thejpster @therealprof @hannobraun I've send y'all invites. Thanks for helping out ❤️

@homunkulus
Copy link
Contributor

📌 Commit bfa0786 has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit bfa0786 with merge 4b1c871...

japaric pushed a commit that referenced this pull request Feb 15, 2018
[RFC] establish a process for adding new traits

This PR establish a process for adding new traits. The details can be found in the diff. This PR is
also proposing using the `embedded-hal-driver` and `embedded-hal-impl` keywords on crates.io to make
implementations of `embedded-hal` and drivers built on top of it more discoverable.

Feel free to suggest changes to these proposals in the comments.

The PR is also adding a list of (WIP) `embedded-hal` implementations and `embedded-hal` drivers. If
your crate is not listed there or if you know of a crate that should be there feel free to send a
follow up PR adding it.

---

Finally, I'd like to use this chance to call for collaborators that will help me with:

- Merging PRs that update the lists in the README

- Triaging issues to make sure that they are properly labeled and that they are following the
  established process.

- "Shepherding" proposals / discussions by preventing them from going off topic and by helping them
  reach a conclusion / consensus, e.g. "Concern $X needs to be addressed before reaching a
  conclusion", "How would this proposal fulfill use case $Y?", etc.

If you'd like to become a collaborator let me know in the comments!
@homunkulus
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

  • This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

@homunkulus
Copy link
Contributor

📌 Commit bfa0786 has been approved by japaric

@homunkulus
Copy link
Contributor

⌛ Testing commit bfa0786 with merge 23ee5d2...

japaric pushed a commit that referenced this pull request Feb 15, 2018
[RFC] establish a process for adding new traits

This PR establish a process for adding new traits. The details can be found in the diff. This PR is
also proposing using the `embedded-hal-driver` and `embedded-hal-impl` keywords on crates.io to make
implementations of `embedded-hal` and drivers built on top of it more discoverable.

Feel free to suggest changes to these proposals in the comments.

The PR is also adding a list of (WIP) `embedded-hal` implementations and `embedded-hal` drivers. If
your crate is not listed there or if you know of a crate that should be there feel free to send a
follow up PR adding it.

---

Finally, I'd like to use this chance to call for collaborators that will help me with:

- Merging PRs that update the lists in the README

- Triaging issues to make sure that they are properly labeled and that they are following the
  established process.

- "Shepherding" proposals / discussions by preventing them from going off topic and by helping them
  reach a conclusion / consensus, e.g. "Concern $X needs to be addressed before reaching a
  conclusion", "How would this proposal fulfill use case $Y?", etc.

If you'd like to become a collaborator let me know in the comments!
@homunkulus
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: japaric
Pushing 23ee5d2 to master...

@homunkulus homunkulus merged commit bfa0786 into master Feb 15, 2018
@ilya-epifanov
Copy link
Collaborator

@japaric I'd be happy to help too!

@japaric japaric deleted the process branch February 16, 2018 18:23
@japaric
Copy link
Member Author

japaric commented Feb 16, 2018

@ilya-epifanov Thanks! I've sent you an invite.

scowcron pushed a commit to scowcron/embedded-hal that referenced this pull request Feb 28, 2018
[RFC] establish a process for adding new traits

This PR establish a process for adding new traits. The details can be found in the diff. This PR is
also proposing using the `embedded-hal-driver` and `embedded-hal-impl` keywords on crates.io to make
implementations of `embedded-hal` and drivers built on top of it more discoverable.

Feel free to suggest changes to these proposals in the comments.

The PR is also adding a list of (WIP) `embedded-hal` implementations and `embedded-hal` drivers. If
your crate is not listed there or if you know of a crate that should be there feel free to send a
follow up PR adding it.

---

Finally, I'd like to use this chance to call for collaborators that will help me with:

- Merging PRs that update the lists in the README

- Triaging issues to make sure that they are properly labeled and that they are following the
  established process.

- "Shepherding" proposals / discussions by preventing them from going off topic and by helping them
  reach a conclusion / consensus, e.g. "Concern $X needs to be addressed before reaching a
  conclusion", "How would this proposal fulfill use case $Y?", etc.

If you'd like to become a collaborator let me know in the comments!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants