Remove redundant Data.Monoid imports #4548
Conversation
As always, a bunch of our customers are lecturers and do not necessarily have |
It is possible to work around the problem by installing binaries directly or via package manager. The package manager should deal with the installation regardless of the pre-installed GHC. The required minimal GHC version for development is more subtle. I remember that someone (I forgot who..) states that the minimal should be the GHC version provided by the stable release of Ubuntu at least. I think this requirement makes sense. |
|
The last stable Ubuntu (bionic) has ghc 8.0.2, but I don't think that that should be a limitation as the ppa repo for Ubuntu ghc always has the latest version anyway. Similarly for MacOS / brew. This was more of an issue in the past when there were various haskell stacks / platforms around, but they seem to have been left by the wayside. The next stable Ubuntu will ship with GHC 8.8.1. |
The release date is 24 April 2020, so it is indeed timely to ask for opinions to remove the support of older GHC versions. |
|
I don't think that I have access to the relevant mailing lists. In any case, we should remember to tweak the various build scripts / configs such as Agda.cabal to capture the supported GHCs. |
|
We shouldn't drop older GHC versions without need. We have customers that are not always on the latest version of OS or GHC. (I am using 8.4 since 8.8 feels slower.) We dropped support for GHC versions when they became too much of a nuisance to maintain; there were major incompatibilities introduced by the AMP and burning-the-bridges proposal, thus it was difficult to satisfy the different GHC versions. We had these discussions before, and the consensus was not to actively remove GHC version support. |
|
|
||
| import qualified Codec.Compression.GZip as G | ||
| import qualified Codec.Compression.Zlib.Internal as Z | ||
|
|
||
| #if __GLASGOW_HASKELL__ >= 804 | ||
| #if __GLASGOW_HASKELL__ >= 821 |
There was a problem hiding this comment.
Was the bound 804 wrong or why did this get changed?
There was a problem hiding this comment.
Also, 821 doesn't mean GHC 8.2.1 but GHC 8.21.
There was a problem hiding this comment.
The bound should be GHC 8.2.1, from my reading of the library dependencies. But I did indeed read the dependency format incorrectly (MIN_VERSION_GLASGOW_HASKELL(x,y,z,z') is the format I was misrememberding. So 804 is really GHC 8.4.x or higher. Since it's merely an optimization I'll revert, assuming that someone found that earlier versions were faulty in some way.
There was a problem hiding this comment.
I removed the hlint.yaml file because I found that it was easier the look at the hlint-report.html file produced by a full hlint (make hlint). This groups the hints nicely. (Previously I had switched various hints on and off one by one.) But it could help to have a default file with the hints that should always be ignored. For example, we have hundreds of redundant brackets and dollars. Some people prefer not to hoist nots. The code base contains a fair fair few x : [] instead of [x]. Some seems to people prefer flips to sections. etc.
There was a problem hiding this comment.
The bound should be GHC 8.2.1, from my reading of the library dependencies.
There was some problem when I tested with GHC 8.2.2.
Since it's merely an optimization I'll revert, assuming that someone found that earlier versions were faulty in some way.
I'd like you to consider that four persons have been involved in this discussion, and we don't have unlimited resources.
src/full/Agda/Utils/Update.hs
Outdated
| @@ -22,6 +22,8 @@ import Control.Monad.Fail (MonadFail) | |||
|
|
|||
| import Control.Monad.Identity | |||
| import Control.Monad.Trans | |||
| -- Control.Monad.Trans.Identity is already exported by Control.Monad.Identity | |||
There was a problem hiding this comment.
But what is the harm?
I think we shouldn't remove support for older GHCs without need.
There was a problem hiding this comment.
This is not a removal but a comment that this can be removed at some future point when GHC 8.0.x is no longer supported.
I think the same.
What did you mean by 'actively'? |
|
I am certainly not advocating dropping support for older versions of GHC; these changes are intended to work with the versions of GHC listed in the cabal file (ie, versions 8.0-8.8). I have, however, annotated code that exists only to satisfy an older compiler / base library. One disadvantage of this approach is that you can end up switching off warnings that arise only from code that is there only for older compilers / libraries. I have not resorted to CPP, unless there is a preexisting CPP directive in a file, as it just adds to compile times, although it seems to be enabled by default(?). But that would be a more explicit approach. |
No. |
|
Hmmm ... the Windows build seems to fail periodically installing backblaze / alex, before it gets to the agda part. |
|
Thanks, @jrp2014. |
Data.Monoidseems to have become part ofbasein 4.8.0.0, which came with 7.10.1, so explicit imports are not needed when using the standard prelude and ghc 8 compilers.I also zapped a couple of other redundant imports while at it.
Data.Semigroupis the next target for removal on the road to warning-free builds. But that needs at least 4.11.0.0, which came with ghc 8.4.1, so maybe something to come back to when we get GHC 8.10.x building Agda.MonadFailis another candidate for elimination, but is likely to need code tweaking as well.