Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

Fix warnings #165

Closed
wants to merge 2 commits into from
Closed

Fix warnings #165

wants to merge 2 commits into from

Conversation

rasendubi
Copy link
Contributor

PR for #159.

@cocreature
Copy link
Collaborator

Thanks! Could you please put the partial type signatures back in and enable in those files {-# OPTIONS_GHC -fno-warn-partial-type-signatures #-}, the types don’t help here.

Also I’ll leave it to @alanz to decide whether we should enable -Werror. We should definitely remove it when we release but until then I like the idea of enabling it.

@rasendubi rasendubi force-pushed the fix_warnings branch 2 times, most recently from 995cd9e to 6cce564 Compare January 9, 2016 20:59
@glittershark
Copy link
Contributor

could have a cabal flag that's off-by-default to have it only enable on travis

This should be removed when releasing or stack support for -Werror is
implemented.
@cocreature
Copy link
Collaborator

A cabal flag sounds like a good idea.

@DanielG
Copy link
Collaborator

DanielG commented Jan 9, 2016

+1 for having a default off flag for -Werror that's enabled on travis. It's that's very annoying during development.

@mgsloan
Copy link
Collaborator

mgsloan commented Jan 9, 2016

Agreed, and a cabal flag is not necessary. This can be done by invoking stack build with --pedantic.

@rasendubi
Copy link
Contributor Author

@mgsloan --pedantic passes -Werror to all underlying libraries which eventually fails to build because of warnings in ghc-mod.

@cocreature
Copy link
Collaborator

Maybe we just need to annoy @mgsloan enough so that he fixes that for us :)

@alanz
Copy link
Collaborator

alanz commented Jan 9, 2016

+1

On Sun, Jan 10, 2016 at 12:02 AM, Moritz Kiefer [email protected]
wrote:

Maybe we just need to annoy @mgsloan https://github.com/mgsloan enough
so that he fixes that for us :)


Reply to this email directly or view it on GitHub
#165 (comment)
.

@mgsloan
Copy link
Collaborator

mgsloan commented Jan 9, 2016

Ah, it isn't that it gets passed to all underlying libraries. Looks like there's a bug with packages that are marked extra-dep: True, where ghc-options are erroneously applied to them.

I didn't know about this bug, and I don't think the stack issue tracker did either. I've opened commercialhaskell/stack#1631 . Feel free to open an issue when you encounter things like that in the future.

@rasendubi
Copy link
Contributor Author

@mgsloan
Copy link
Collaborator

mgsloan commented Jan 9, 2016

Gotcha. I knew the issue sounded familiar! Didn't find it since I was searching for ghc-options rather than pedantic

@cocreature
Copy link
Collaborator

So what do we do here? It seems like the options are either adding a flag or removing the -Werror. I don’t really care what we decide to do, but I don’t want this PR to sit around waiting just because of that reason.

@glittershark
Copy link
Contributor

I'm ++ for the flag

@alanz
Copy link
Collaborator

alanz commented Jan 11, 2016

Perhaps remove -Werror, and then build with --pedantic once the bug is
fixed there

On Mon, Jan 11, 2016 at 9:45 PM, Griffin Smith [email protected]
wrote:

I'm ++ for the flag


Reply to this email directly or view it on GitHub
#165 (comment)
.

@cocreature
Copy link
Collaborator

Since we are now back to having two options and two people each favoring one of those, let’s just go with a flag for now so we can merge this ( @rasendubi please add that). Once --pedantic is actually usable again, we can kill it.

@alanz
Copy link
Collaborator

alanz commented Jan 11, 2016

ok

@cocreature cocreature mentioned this pull request Jan 13, 2016
@cocreature
Copy link
Collaborator

Closing in favor of #171

@cocreature cocreature closed this Jan 13, 2016
@alanz alanz added this to the prehistory milestone Feb 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants