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

Add 'warnings' field to configuration #91

Closed
chshersh opened this issue Jun 8, 2018 · 7 comments
Closed

Add 'warnings' field to configuration #91

chshersh opened this issue Jun 8, 2018 · 7 comments
Assignees
Labels

Comments

@chshersh
Copy link
Contributor

chshersh commented Jun 8, 2018

There're some warnings which are not included in -Wall but some people think they should be used anyway. I think we should create special configuration field for this. And add some fields to our default config.

Source:

Comments are interesting as well.

@chshersh chshersh added this to the v1.1 Upgrade milestone Jun 8, 2018
@chshersh
Copy link
Contributor Author

I propose to add the following warnings by default:

  • -Wincomplete-uni-patterns
  • -Wincomplete-record-updates
  • -Wmissing-import-lists
  • -Wcompat
  • -Widentities
  • -Wredundant-constraints (since 8.0)
  • -Wmissing-export-lists (since 8.4.1)
  • -Wpartial-fields (since 8.4.1)

List of all GHC warnings can be found here:

@chshersh
Copy link
Contributor Author

chshersh commented Aug 3, 2018

@willbasky This task is about only adding warnings fields to configuration. You should this extra field to configuration here:

-- | Potentially incomplete configuration.
data ConfigP (p :: Phase) = Config
{ cOwner :: p :- Text
, cFullName :: p :- Text
, cEmail :: p :- Text
, cLicense :: p :- License
, cGhcVer :: p :- [GhcVer]
, cCabal :: Decision
, cStack :: Decision
, cGitHub :: Decision
, cTravis :: Decision
, cAppVey :: Decision
, cPrivate :: Decision
, cScript :: Decision
, cLib :: Decision
, cExe :: Decision
, cTest :: Decision
, cBench :: Decision
, cPrelude :: Last CustomPrelude
, cExtensions :: [Text]
} deriving (Generic)

And then you should use it to populate ghc-options field in .cabal files.

@willbasky
Copy link
Collaborator

willbasky commented Aug 3, 2018

Will warnings add to configuration if there is --file conf.toml or\and if there is user's yes in interactive mode?

@chshersh
Copy link
Contributor Author

chshersh commented Aug 3, 2018

@willbasky We have this note in README about priority order:

But personally I think that it doesn't make much sense to ask about warnings interactively. We already add -Wall by default and this should be enough in 99.9% cases. For those who want extra warnings, it's better to specify them in configuration because names on warnings are very long. And it's not convenient to type them manually. So I expect that nobody will write warnings in interactive mode.

But I think that it's a good idea to be able to specify them through CLI options.

@willbasky
Copy link
Collaborator

I agree with you. If someone will want to change warnings it easy to rewrite them in text mode after project created.

@vrom911
Copy link
Member

vrom911 commented Aug 3, 2018

I don't entirely agree about CLI part. I guess it's almost the same as with pragmas, better to use config files for that.

@chshersh
Copy link
Contributor Author

chshersh commented Aug 3, 2018

@vrom911 Yeah, that makes sense as well. Probably it's better to not allow them as CLI as well.

willbasky added a commit that referenced this issue Aug 3, 2018
chshersh pushed a commit that referenced this issue Aug 6, 2018
* [#91] Add warning fields

* Add 3 warnings (ghc > 8)

* Refactor

* Refactor versionWarnings

* Fix warnings

* Fix hasLeast

* Fix readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants