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

Backport instances introduced in base-4.14 #51

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Backport instances introduced in base-4.14 #51

merged 1 commit into from
Jan 27, 2020

Conversation

RyanGlScott
Copy link
Member

Fixes #49.

@RyanGlScott
Copy link
Member Author

I'm rather conflicted on whether this change deserves a major version bump or a minor version bump.

  • On one hand, base-orphans is a package that contains nothing but orphan instances. There is some precedent for only giving minor version bumps to such packages when they add new orphan instances.
  • On the other hand, this adds quite a lot of instances for commonly used data types. Some of these instances, in fact, are already defined in other packages, such as fclabels (here) and parameterized-utils (here)! As far as I can tell, neither of these libraries run the risk of clashing with base-orphans at present, but the danger is there. (Either way, I plan to submit PRs to both libraries after the next base-orphans release to make them use base-orphans.)

What are your thoughts, @phadej?

@phadej
Copy link
Contributor

phadej commented Jan 27, 2020

I'd say that base-orphans is de-facto place for base orphans, and if some packages break we (trustees) have a special power to add direct dependency to base-orphans to avoid orphans clashing.

And even major bump won't prevent from diamond problem

         base
     /       \
fclabels    base-orphans
    |             | 
    |            foo
    |             |
     \           /
   something which uses orphan

As soon as foo allows never base-orphans, something which uses orphan will break.
Technically, foo should also make a major update when using base-orphans,
so IMHO it's better that we treat base-orphans specially.

So, either play by the rules, or change the rules: haskell/pvp#3

@RyanGlScott
Copy link
Member Author

Just to make sure I understand your position, do you think a minor version bump would be acceptable here? I'm not entirely sure what you mean by "play by the rules".

@phadej
Copy link
Contributor

phadej commented Jan 27, 2020

I think no-one treats instances in base-orphans as understood by PVP.

Breaking change. If any entity was removed, or the types of any entities or the definitions of datatypes or classes were changed, or orphan instances were added or any instances were removed, then the new A.B MUST be greater than the previous A.B. Note that modifying imports or depending on a newer version of another package may cause extra orphan instances to be exported and thus force a major version change.

So I think it doesn't make sense to obey MUST here, as at large it's no obeyed down-stream either.

That said, if fclabels and parametrized-utils will break with newer base, they should rather migrate to use base-oprhans than #ifdef their instances. Otherwise bad things happen.

@RyanGlScott
Copy link
Member Author

So I think it doesn't make sense to obey MUST here, as at large it's no obeyed down-stream either.

In that case, I'll plan to make the next base-orphans release be 0.8.2 rather than 0.9.

That said, if fclabels and parametrized-utils will break with newer base, they should rather migrate to use base-oprhans than #ifdef their instances. Otherwise bad things happen.

Indeed. I have PRs for each library ready to go once base-orphans-0.8.2 is out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backport new instances introduced in GHC 8.10
2 participants