search

alpmestan / servant

Moved to http://github.com/haskell-servant
50 stars 1 forks source link

Remove all ghc warnings. #38

Open soenkehahn opened 9 years ago

soenkehahn commented 9 years ago

And establish consistent warning settings for the library and the test-suite.

soenkehahn commented 9 years ago

@alpmestan: I know you have a different habit. Are you ok with this?

alpmestan commented 9 years ago

Yeah. While I think -Werror is nice to have right before releasing, I'm not fond of having it during development. It mostly just made a bunch of travis builds fail (because of warnings, not because of actual problems). So I'm not sold on -Werror before we get close to a release, but aside from that I don't mind the occasional fno-warn-blah, I won't nitpick on whatever warnings you guys usually like to have.

shlevy commented 9 years ago

FWIW, from the perspective of a distro package maintainer I hate -Werror being in the build scripts end users are meant to use. This is less of a concern given the GHC monoculture for haskell, but even then, the assumption that your version of the toolchain will have the exact same set of warnings as all of your users' (and no one actually tests more than their own toolchain...) just causes pain to no good end I've ever seen.

So, IMO devs should locally verify -Werror passes before publishing a new version, but should not put that into the cabal file.

alpmestan commented 9 years ago

Fair point indeed. Releasing it with -Werror isn't a good idea.

The main point I wanted to emphasize was that I don't like it when I see a bunch of warnings while I build stuffs, so I'd like for servant to be warning-free when people build it (even if we're actually silencing some (non-important) warnings by hand).

jkarni commented 9 years ago

As a data point, cabal check complains (and exits non-zero) with -Werror.

Maybe just Travis on master should compile with Werror?

alpmestan commented 9 years ago

Well... Right now, the master branch is quite "useless". If you meant new-impl, then this may get in our way more than it would help us, since we don't really care about most warnings when we are toying around to see how our ideas play out, or when we're just beginning to shape up some new combinator etc.

I guess we can just make sure that servant builds with -Werror locally before any release, like Shea suggests.

soenkehahn commented 9 years ago

+1 for using the same settings on travis and in the cabal file. And +1 for omitting -Werror.