-
-
Notifications
You must be signed in to change notification settings - Fork 388
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
Fix -Wall and -Wunused-packages in explicit-record-fields plugin #3996
Fix -Wall and -Wunused-packages in explicit-record-fields plugin #3996
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
, containers | ||
, aeson | ||
hs-source-dirs: src | ||
default-language: Haskell2010 | ||
|
||
if flag(pedantic) | ||
ghc-options: -Werror | ||
-Wwarn=incomplete-record-updates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhrcek Now that I look into it, I think the reason it was specified this way was that the plugin is making use of incomplete record updates. The failing test on CI shows where that happens. Off the top of my head I can't think of a straightforward way to eliminate it, as it happens while modifying the AST given by GHC. Maybe you have come across a better way to do this while fixing these warnings for the other plugins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically, this section of GHC options means that "We want warnings to be promoted to errors (-Werror
), except for the incomplete record updates warning". Unless there is a compelling reason to change this, I think this is a sensible setting for this specific plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GHC options on specific files take priority, so I'd be inclined to put the -Wwarn
option on the module with the problem and add a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the semantic of -Werror and how -Wwarn turns it into warning only. I just wasn't sure why it's here (because cabal build --flag=pedantic for this plugin didn't throw any error.
Let me put it back for now and deal with fixing these kinds of warnings separately later.
…kell#3996) * Fix -Wall and -Wunused-packages in explicit-record-fields plugin * Don't remove -Wwarn=incomplete-record-updates --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
No description provided.