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

Disable fatal warnings in the library #9269

Merged
merged 1 commit into from Oct 23, 2020

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Oct 23, 2020

The fatal warnings PR (for library) was merged prematurely as it is
impacting my exhaustivity PR (1). Rather do anything (like rebase or tweak)
to the 12 commits that my PR consists of and therefore causing CI to
rebuild each commit, I'm separately going to PR disabling fatal
warnings, so I can merge my PR without it instantly breaking the 2.13.x
branch.

I'll do the same for the reflect and compiler PRs, and once we've
re-STARR'd we can re-enable fatal warnings by either resolving all the
exhaustivity warnings or opting out of them (or a mix of both).

cc @NthPortal

The fatal warnings PR (for library) was merged prematurely as it is
impacting my exhaustivity PR.  Rather do anything (like rebase or tweak)
to the 12 commits that my PR consists of and therefore causing CI to
rebuild each commit, I'm separately going to PR disabling fatal
warnings, so I can merge my PR without it instantly breaking the 2.13.x
branch.

I'll do the same for the reflect and compiler PRs, and once we've
re-STARR'd we can re-enable fatal warnings by either resolving all the
exhaustivity warnings or opting out of them (or a mix of both).
@scala-jenkins scala-jenkins added this to the 2.13.4 milestone Oct 23, 2020
@dwijnand dwijnand merged commit 59c6eda into scala:2.13.x Oct 23, 2020
@dwijnand dwijnand deleted the library-disable-fatal-warnings branch October 23, 2020 10:56
@NthPortal
Copy link
Contributor

I would rather have temporarily disabled exhaustivity warnings via -Wconf than disabled fatal warnings entirely

@NthPortal
Copy link
Contributor

NthPortal commented Oct 23, 2020

@dwijnand I am actually significantly concerned that this will lead to backsliding in unrelated areas. Can we instead change library (and the other modules) to have "-Wconf:msg=match may not be exhaustive:i"? (or w if that overrides -Werror, cc @som-snytt)

@dwijnand
Copy link
Member Author

Yep, didn't think of that, thought we were blocked on re-STARR'ing and having access to the opt-outs. 👍

@SethTisue SethTisue added the internal not resulting in user-visible changes (build changes, tests, internal cleanups) label Oct 23, 2020
@som-snytt
Copy link
Contributor

You two are talking way over my head. It should be called WAP for Warning Aware Programming.

Have you considered just using -nowarn? The question is whether to use -nowarn during development because warnings are noisy and annoying, or during PR validation because warnings are only for guiding development and they are too noisy and annoying for validation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal not resulting in user-visible changes (build changes, tests, internal cleanups)
Projects
None yet
5 participants