Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only use -Werror in CI #428

Merged
merged 18 commits into from
Jan 17, 2020
Merged

Only use -Werror in CI #428

merged 18 commits into from
Jan 17, 2020

Conversation

robrix
Copy link
Contributor

@robrix robrix commented Jan 16, 2020

This PR adds new cabal.project files specifically for CI builds, allowing us to enable -Werror (“hard mode”) only in CI.

@@ -1,3 +1,6 @@
-- ATTENTION: care must be taken to keep this file in sync with cabal.project.ci. If you add a package here, add it there (and add a package stanza with ghc-options to enable errors in CI at the bottom of that file).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These notices are important, and a rather unfortunate necessity: We can’t just pass --ghc-option=-Werror to cabal in CI, because a long-standing bug in cabal causes it to apply ghc-options specified on the CLI to all packages instead of only the local ones.

We could perhaps have added cabal.project.local files specifying -Wwarn for all local packages, but we’d have had to keep those in sync too, and as that file is the target of cabal configure, it would potentially introduce a bunch of work to keep spurious changes to those files out of PRs.

On the balance, this seemed a better solution, albeit unfortunately duplicative; hopefully the ATTENTION notices in these files will remind us to keep things synced up. (And if not, maybe we can add a CI check that cabal.project is a prefix of cabal.project.ci.)

Comment on lines +66 to +70
package semantic-tsx
ghc-options: -Werror

package semantic-typescript
ghc-options: -Werror
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were actually already running into trouble keeping these things synced up correctly, as neither of these packages had -Werror in CI until now.

@robrix robrix merged commit 908cca2 into master Jan 17, 2020
@robrix robrix deleted the game-over,-man!-game-over! branch January 17, 2020 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-Werror is inconvenient for local builds
2 participants