👍 Looks good, thanks!

Any thoughts on why CI is breaking?

Looking at the log, it's because there seem to be a ton of warnings like Redundant constraint: Functor m now. I guess that's new in ghc-8.0.0.20160229.

Unfortunately Monad only became a subclass of Applicative in base-4.8.0.0 (that is, GHC 7.10), so we'd have to drop support for GHC 7.6 and 7.8 if we wanted to remove those constraints. Or put CPP everywhere (yuck). Just using base-compat isn't enough, as it re-exports class definitions from base.

I guess the options are, modify the cabal file to ignore those warnings, or remove them and stop supporting GHC < 7.10. I'd vote to drop support for older GHCs, as I've found these warnings can be very useful.

As long as we're using Stack to build (which I am now, for example, for the Mac binaries), then I'm fine with dropping < 7.10. Does the Linux64 binary build use Stack?

I guess we can't build with GHC8 using Stack/Stackage right now.

I'd really like to have CI working again, so maybe disabling the GHC8 build makes the most sense for now.

Thanks!

This actually breaks purescript-aff since the Par module has some incorrect indentation on line 50. We'll have to remember to fix that when we release this.

This actually breaks purescript-aff

Milestone 0.9?

Possibly, yes.

Just to make sure, is it an error in the Par module or in the compiler. If it's in the module I can make a PR now to fix it.

Hmm, we try not to release breaking changes in the 0.x.y releases, maybe we'd be better off reverting this until 0.9? I'm not sure.

I think aff is common enough that we should probably hold this off until 0.9. While it's not technically a breaking change, it is in the sense that we don't have a specification for indentation written down anywhere, so "it is what it does" right now 😄

@DavidLindbom I've reverted this, but if you'd like to reopen it, we can merge it in for 0.9.

Thanks!

I can open it again but I just want to make sure first if this should be the correct behavior? More exact should that line in purescript-affbe disallowed or should I try to rework the patch?

I think aff needs to be changed either way, but it would probably be a major bump for both compiler and the package itself.

It wasn't immediately obvious to me what's wrong with the indentation, but I think I see it now:

 instance altPar :: Alt (Par e) where
  alt (Par a1) (Par a2) =
    let maybeKill va ve err =
      do e <- takeVar ve
         if e == 1 then killVar va err else return unit
         putVar ve (e + 1)
    in Par do
      va <- makeVar     -- the `a` value
      ve <- makeVar' 0  -- the error count (starts at 0)
      c1 <- forkAff $ attempt a1 >>= either (maybeKill va ve) (putVar va)
      c2 <- forkAff $ attempt a2 >>= either (maybeKill va ve) (putVar va)
      (takeVar va) `cancelWith` (c1 <> c2)

Should it be

    let
      maybeKill va ve err =
        do e <- takeVar ve
           if e == 1 then killVar va err else return unit
           putVar ve (e + 1)

instead? I happened to notice we have a do with indentation that way in the tests today, so I assume it's let related instead?

Oh I missed it too that's exactly right! That was #1848.

The first lexeme after let defines a new indentation level, and similarly for do. The issue is just that the do is to the left of the let (or the maybeKill is to the right of the let, depending on how you look at it).