search

con-kitty / categorifier

Interpret Haskell programs into any cartesian closed category.
BSD 3-Clause "New" or "Revised" License
57 stars 2 forks source link

Use -Werror in local builds and CI #78

Closed sellout closed 2 years ago

sellout commented 2 years ago

This doesn’t affect the cabal files, so published packages still won’t fail on warnings.

sellout commented 2 years ago

Can we not use -Werror in local builds, or at least have an easy way to turn it off (like some cli arguments to nix develop)?

The elusive ideal, of course, is having 1. warnings that don't disappear on subsequent builds and 2. the failure only occur at the end of the build.

Using -Werror breaks 2, but leaving it off breaks 1.

We should make it toggleable for local builds, after which I have a slight preference for the default being “on”, to avoid surprises during CI.

However, I wouldn't want to hold up this change for that functionality. We currently have a less elegant toggle by commenting out a line of flake.nix and cabal builds also don't have -Werror locally.

zliu41 commented 2 years ago

surprises during CI.

That has definitely happened to me before, but much less frequently than me having to turn off -Werror during development. But once we have a toggle for it then it won't matter to me what the default is.

zliu41 commented 2 years ago

Also it occurred to me that when supporting many GHC versions, it can be quite hard to eliminate all warnings.

sellout commented 2 years ago

Also it occurred to me that when supporting many GHC versions, it can be quite hard to eliminate all warnings.

Yeah, there’s a bit of annoyance with definitions moving between GHC API modules, but I think it's worth it, because allowing any warnings makes it easier to miss more important ones. E.g., there were some partial pattern matches and shadowed identifiers that were already missed. Plus, the most common GHC versioning warning is unused imports, and there are some of those that should actually be cleaned up.

sellout commented 2 years ago

Can we not use -Werror in local builds, or at least have an easy way to turn it off (like some cli arguments to nix develop)?

So, this change doesn't yet enable -Werror in cabal builds, only in Nix builds. So local builds still generally avoid it. Think we can go ahead with it?