snoyberg / mono-traversable

Type classes for mapping, folding, and traversing monomorphic containers
155 stars 67 forks source link

classy-prelude: Support time >= 1.10 #205

Closed TeofilC closed 3 years ago

TeofilC commented 3 years ago

This adds support for time >= 1.10 by removing parseTime from the re-exports of ClassyPrelude, which was removed in that version. I've bumped the version number and added a Changelog entry.

Alternatively:

Fixes #204

andreasabel commented 3 years ago

Did you consider parseTime = parseTimeM True under some #if? (Concerning the change in time: Frankly, I don't understand why one would remove parseTime and ask everyone downstream to replace it by parseTimeM True.)

TeofilC commented 3 years ago

I'd be happy to change this MR to do that instead

andreasabel commented 3 years ago

I'd be happy to change this MR to do that instead

As I am no user of classy-prelude, I have (and should not have) an influence on this. Let @snoyberg or @gregwebs decide.

snoyberg commented 3 years ago

That's a great suggestion @andreasabel, I'd prefer such a non-breaking change.

TeofilC commented 3 years ago

I pushed a version that implements a compatibility shim

ocharles commented 2 years ago

Did this release go out OK? I'm still struggling to get classy-prelude-1.5.0.1 to build:

Configure flags:
--prefix=/nix/store/iid4kni05dml7waf93d1pbhxkh12ki97-classy-prelude-lib-classy-prelude-1.5.0.1 lib:classy-prelude --package-db=clear --package-db=/nix/store/0m6zadik47arzqhkasrirfgm0n2yj7ps-classy-prelude-lib-classy-prelude-1.5.0.1-config/lib/ghc-9.2.1/package.conf.d --exact-configuration --dependency=async=async-2.2.4-DkCLaCwah46DnmGpHEGuZ7 --dependency=basic-prelude=basic-prelude-0.7.0-3vxbeEWKByM62C4zLmmYxX --dependency=bifunctors=bifunctors-5.5.11-LVfCqd4lzxLCo7SRFZ5N0P --dependency=chunked-data=chunked-data-0.3.1-8DlrMJGu6JEHAqMlWAV8Yo --dependency=dlist=dlist-1.0-4sH8SERK9Ol4NXTXrCJMDs --dependency=hashable=hashable-1.4.0.1-Cf1Pmot4a2h8AHo6mmqzyu --dependency=mono-traversable=mono-traversable-1.0.15.3-7whdO1jppfACPhBa8kc8nd --dependency=mono-traversable-instances=mono-traversable-instances-0.1.1.0-5Ab589sdyEjFcI1WA923Vp --dependency=mutable-containers=mutable-containers-0.3.4-CUPwDZ5zO0OFLPExDAm0Ha --dependency=primitive=primitive-0.7.3.0-LGi6raRr8n2AUxTlPBJxpd --dependency=say=say-0.1.0.1-3Q7JvJpnfYFhRcT06Fn8z --dependency=stm-chans=stm-chans-3.0.0.6-Dpt4kVo9Vlb9T3QyMu0nSj --dependency=unliftio=unliftio-0.2.20-LbSXcoqftKBIcps9A2HNWU --dependency=unordered-containers=unordered-containers-0.2.15.0-2b9bfpgDECzI3IDQBqzBoc --dependency=vector=vector-0.12.3.1-9U2CVpmX0JH3q0DtIzQrRS --dependency=vector-instances=vector-instances-3.4-CB1KBXP32VJIqhka4Z295g --dependency=rts=rts --dependency=ghc-heap=ghc-heap-9.2.1 --dependency=ghc-prim=ghc-prim-0.8.0 --dependency=integer-gmp=integer-gmp-1.1 --dependency=base=base-4.16.0.0 --dependency=deepseq=deepseq-1.4.6.0 --dependency=array=array-0.5.4.0 --dependency=ghc-boot-th=ghc-boot-th-9.2.1 --dependency=pretty=pretty-1.1.3.6 --dependency=template-haskell=template-haskell-2.18.0.0 --dependency=ghc-bignum=ghc-bignum-1.2 --dependency=exceptions=exceptions-0.10.4 --dependency=stm=stm-2.5.0.0 --dependency=ghc-boot=ghc-boot-9.2.1 --dependency=ghc=ghc-9.2.1 --dependency=Cabal=Cabal-3.6.0.0 --dependency=array=array-0.5.4.0 --dependency=binary=binary-0.8.9.0 --dependency=bytestring=bytestring-0.11.1.0 --dependency=containers=containers-0.6.5.1 --dependency=directory=directory-1.3.6.2 --dependency=filepath=filepath-1.4.2.1 --dependency=ghc-boot=ghc-boot-9.2.1 --dependency=ghc-compact=ghc-compact-0.1.0.0 --dependency=ghc-prim=ghc-prim-0.8.0 --dependency=hpc=hpc-0.6.1.0 --dependency=mtl=mtl-2.2.2 --dependency=parsec=parsec-3.1.14.0 --dependency=process=process-1.6.13.2 --dependency=text=text-1.2.5.0 --dependency=time=time-1.11.1.1 --dependency=transformers=transformers-0.5.6.2 --dependency=unix=unix-2.7.2.2 --dependency=xhtml=xhtml-3000.2.2.1 --dependency=terminfo=terminfo-0.4.1.5 --with-ghc=ghc --with-ghc-pkg=ghc-pkg --with-hsc2hs=hsc2hs --with-gcc=cc --with-ld=ld.gold --ghc-option=-optl-fuse-ld=gold --ld-option=-fuse-ld=gold --with-ar=ar --with-strip=strip --disable-executable-stripping --disable-library-stripping --enable-library-profiling --disable-executable-profiling --enable-static --enable-shared --disable-coverage --enable-library-for-ghci --profiling-detail=default --enable-split-sections 
Configuring library for classy-prelude-1.5.0.1..
Warning: The flag --disable-executable-profiling is deprecated. Please use
--disable-profiling instead.
building
Preprocessing library for classy-prelude-1.5.0.1..
Building library for classy-prelude-1.5.0.1..
[1 of 2] Compiling ClassyPrelude    ( src/ClassyPrelude.hs, dist/build/ClassyPrelude.o, dist/build/ClassyPrelude.dyn_o )

src/ClassyPrelude.hs:632:6: error:
    Not in scope: type constructor or class ‘ParseTime’
    Perhaps you want to add ‘ParseTime’ to the import list
    in the import of ‘Data.Time’
    (src/ClassyPrelude.hs:(190,1)-(202,5)).
    |
632 |   :: ParseTime t
    |      ^^^^^^^^^

src/ClassyPrelude.hs:633:6: error:
    Not in scope: type constructor or class ‘TimeLocale’
    Perhaps you want to add ‘TimeLocale’ to the import list
    in the import of ‘Data.Time’
    (src/ClassyPrelude.hs:(190,1)-(202,5)).
    |
633 |   => TimeLocale -- ^ Time locale.
    |      ^^^^^^^^^^

Note in the configure flags I'm using time-1.11, and you can see I'm building classy-prelude-1.5.0.1.

ocharles commented 2 years ago

Ah, I think it's because ParseTime and TimeLocale aren't conditionally imported.

TeofilC commented 2 years ago

Oh dear, I must not have tested building this with the type signature.

I'll make a new PR to fix this unless you're already making one

ocharles commented 2 years ago

@TeofilC please go ahead, I'm not currently working on a fix

andreasabel commented 2 years ago

The meta-question: Why didn't CI catch this?

TeofilC commented 2 years ago

The meta-question: Why didn't CI catch this?

I think CI doesn't test with time > 1.10. It looks like the latest version on lts-18 is 1.9.3 and even nightly only has that. https://www.stackage.org/package/time/snapshots

snoyberg commented 2 years ago

Would it be possible to add an additional stack.yaml that has a time in extra-deps that demonstrates the patch is working?