Skip to content

[Discussion] the future of Unproven traits and practices for new and breaking changes #99

Closed
@ryankurte

Description

@ryankurte
Contributor

This issues is to discuss our management of breaking and non-breaking changes in the 0.x series of releases, to limit the scope of the discussion I suggest this is revised before we stabilise.

As mentioned in #97, the unproven flag on traits aren't working quite as we might intend.
We've also put forward an feature-flag approach to mitigate the impact of breaking changes to hal traits on library users that I propose we adopt for all future changes.

Please refer to #97 for an introduction to this approach and previous discussion about the unproven flag.

cc. @hannobraun @therealprof @eldruin

Activity

self-assigned this
on Oct 2, 2018
therealprof

therealprof commented on Oct 3, 2018

@therealprof
Contributor

@ryankurte Maybe you should should lay out what the vision is regarding the feature-flag approach and create an RFC for the changes this requires.

Regarding the breaking and non-breaking changes and unproven flag: IMHO we should encourage people to create pull requests removing the unproven feature gate by providing references to known implementations which provide and/or use the unproven trait in question. We'll verify those claims, accept the PR, bump the version and release a new version. For breaking changes we should announce the upcoming change to solicit for more breaking changes and then do the same.

However I'm not quite convinced we really need to cause breakage for #97. I'd rather have separate traits for the fallible versions. That way people can easily adjust and bridge between the two versions instead of breaking everything in one swoop.

hannobraun

hannobraun commented on Oct 3, 2018

@hannobraun
Member

@therealprof

Regarding the breaking and non-breaking changes and unproven flag: IMHO we should encourage people to create pull requests removing the unproven feature gate by providing references to known implementations which provide and/or use the unproven trait in question. We'll verify those claims, accept the PR, bump the version and release a new version. For breaking changes we should announce the upcoming change to solicit for more breaking changes and then do the same.

I agree. If we decide to keep the unproven flag, we should certainly do that.

However I'm not quite convinced we really need to cause breakage for #97. I'd rather have separate traits for the fallible versions. That way people can easily adjust and bridge between the two versions instead of breaking everything in one swoop.

It's not clear to me whether you mean we should have a second set of traits as a transition strategy or permanently.

If you mean adding a second set of traits as a transition strategy, we already discussed this on #95 and #97. I don't remember the details right now, but we agreed the approach currently taken by the pull request is the better one.

If you mean we should add a second set of traits permanently, I strongly disagree. Citing my own comment from #95:

An implementation that has to deal with errors has three options regarding what to do about the infallible traits:

  1. Implement them and panic on errors. Bad, because drivers will not expect the panics.
  2. Implement them and ignore errors. Worse, because now we have problems that are just as unpredictable but less obvious.
  3. Not implement them. The only sane choice, but it will mean that drivers that are built on the infallible traits won't run on that platform.

Given those options, we can't recommend that drivers use the infallible traits, as it will either cause them problems or restrict them in their portability. But since they're simpler than the fallible ones, many drivers probably will use them.

I think that leaves us with two viable options:

  1. Add the error type and make people deal with the breakage and additional complexity.
  2. Decide that embedded-hal just doesn't support cases where pin operations could fail.

I prefer option 1.

therealprof

therealprof commented on Oct 3, 2018

@therealprof
Contributor

If you mean adding a second set of traits as a transition strategy,

Yes, as a transition strategy.

hannobraun

hannobraun commented on Oct 3, 2018

@hannobraun
Member

Yes, as a transition strategy.

Okay, understood. I'm open to discussing the transition strategy, especially since I recently realized we need to add those additional feature flags to the Travis build, then remember to remove them again later. Overall, I don't feel strongly about the transition strategy, as long as the end result is good.

changed the title [-]The future of `Unproven` traits and practices for new and breaking changes[/-] [+][discussion] the future of `Unproven` traits and practices for new and breaking changes[/+] on Oct 4, 2018
changed the title [-][discussion] the future of `Unproven` traits and practices for new and breaking changes[/-] [+][Discussion] the future of `Unproven` traits and practices for new and breaking changes[/+] on Oct 4, 2018
ryankurte

ryankurte commented on Oct 4, 2018

@ryankurte
ContributorAuthor

I've opened an RFC for mitigation of breaking changes in #100.

If we don't have a particular problem with unproven except for it's prevalence / longevity, I propose closing this issue in favour of a Prove unproven traits meta issue to work on removing them, any agreement / disagreement?

therealprof

therealprof commented on Oct 5, 2018

@therealprof
Contributor

Indeed, my only beef with unproven is the longevity. So +1 from me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @hannobraun@ryankurte@therealprof

      Issue actions

        [Discussion] the future of `Unproven` traits and practices for new and breaking changes · Issue #99 · rust-embedded/embedded-hal