Set deny-warnings = false in contributor defaults

[jyn514]

This makes it easier to make rapid changes without having to constantly
fix warnings.

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[nagisa]

@bors r+

[bors]

📌 Commit b22e039 has been approved by nagisa

[joshtriplett]

@jyn514 If we're going to change this default, can we make sure there's some indication at the end of a build that a warning occurred, so it doesn't get lost?

[jyn514]

@joshtriplett rustc will already print 'x warnings generated' at the end, won't it? Are you asking to collect all of those messages per-crate together into a single message? That seems like it would need cargo support.

[joshtriplett]

@jyn514x warnings generated is fine, as long as that appears in the last few lines of output from ./x.py build or ./x.py test or similar. I wouldn't want it to get pushed off the screen by other builds.

[jyn514]

Currently, rustc will print that once for each crate that's built, but like you said it might get pushed off the screen if there are a lot of crates. Printing the sum at the very end sounds useful but I'd rather not try to parse diagnostics. I think cargo already uses --error-format=json --json=diagnostic-rendered-ansi, could we add this in cargo instead?

[joshtriplett]

@jyn514 I'd be fine with that, but I think that needs to happen before allowing warnings by default, or contributors will miss warnings entirely.

[jyn514]

CI denies warnings, so it will be picked up by the test suite. Are you worried people will push changes without looking at the warnings and have to spend time fixing it after the fact? That's a valid concern but it's no worse than forgetting to run x.py fmt IMO.

[jyn514]

Also, to be clear - all of these defaults are opt-in, not opt-out. Even if you opt-in with profile = "compiler" you can always override specific settings in config.toml.

[joshtriplett]

On Sat, Oct 03, 2020 at 04:35:38PM -0700, Joshua Nelson wrote: CI denies warnings, so it will be picked up by the test suite. Are you worried people will push changes without looking at the warnings and have to spend time fixing it after the fact?

Yes, and further that they'll feel like they would have preferred to fix them before submitting but not have known to. That can be stressful for contributors, especially new contributors.

[jyn514]

Okay, that's reasonable.

@bors r-

[jyn514]

Blocked on rust-lang/cargo#8749

[bors]

☔ The latest upstream changes (presumably #77606) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[jyn514]

I am not planning to follow up on the cargo side of this.