-
Notifications
You must be signed in to change notification settings - Fork 182
Bump to GHC 8.0 #812
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
Bump to GHC 8.0 #812
Conversation
0bc9f7f
to
2becf6e
Compare
containers/src/Data/IntMap.hs
Outdated
|
||
-- | This function is being removed and is no longer usable. | ||
-- Use 'Data.IntMap.Strict.insertWith' | ||
insertWith' :: Whoops "Data.IntMap.insertWith' is gone. Use Data.IntMap.Strict.insertWith." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While bumping lower bound is just a maintenance thing, the removal of functions is a breaking change.
Why you do them at the same time? Is it time for containers-0.7
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@phadej I was just thinking in terms of getting rid of cruft. Are you opposed to the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New idea! Let's delete those without a major release. I argue that's okay. The functions were totally disabled anyway, so the disruption of removing them is limited to potentially having to delete them from hiding
clauses. This is no worse than the potential disruption of adding functions, which is allowed in minor releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should removed them in containers-0.6
. Removal of symbols whenever they are usable/useful or not is a breaking change, period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concrete and not too contrived example: Someone could still explicitly-re-export these symbols, removal would break that "use"-site.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a bit too much going on to review this properly. I think it would be better to do the removal of deprecated things in a separate PR.
What I've seen so far looks reasonable though.
@sjakobi, I reverted the removal of deprecated things. Do you think you can review it now? |
* Raise minimum GHC version to 8.0 so we'll be able to define `Lift` instances, avoid certain conditional definitions, etc. * Raise Cabal version for tests to 2.2; use `common` stanzas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! :)
Yay! |
Raise minimum GHC version to 8.0 so we'll be able to define
Lift
instances, avoid certain conditional definitions, etc.Raise Cabal version for tests to 2.2; use
common
stanzas.