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
Set default value deny-warnings
for compiler profile to false
#124439
base: master
Are you sure you want to change the base?
Set default value deny-warnings
for compiler profile to false
#124439
Conversation
rustbot has assigned @albertlarsan68. Use |
This PR modifies If appropriate, please update |
i strongly second this, i find deny-warnings extremely frustrating and it is quite possible that not knowing how to turn it off for a bit longer would have made me stop contributing. i think we should strive to have the same defaults here as rust in general, and there's a good reason why in a default cargo projects, warnings aren't denied by default. |
doing it just in the compiler profile is a bit silly though, it should be the default for everyone |
I'm pretty against this. We definitely shouldn't allow PRs to be merged with outstanding warnings, so of course, in practice this will just move errors from local builds to CI. |
@compiler-errors so you use and like |
Yep -- it definitely catches a significant number of logical errors that I would otherwise ignore if I saw a string of warnings being emitted during my build. Especially bc I usually chain Also I'm not certain whether "default for everyone" also includes allowing warnings to pass the final bors-merge and make it to master -- I assume it doesn't. |
it would definitely not involve making it pass CI, denying it in CI is very important :D |
yeah, I think this would be the best compromise. Something that explains the tradeoff (and also warns that if they disable them locally then they may fail CI, so still fix them before they put up a PR). |
Also, the build process is quite verbose, so if you leave the build in the background, the warnings may be way up in the terminal, but I can see the annoyance of some of the "less important" warnings, as the "mut var not mutated", for which fixing it can mean rebuilding the whole compiler, which can be long. Why not make discuss it at the next T-compiler meeting? (not really sure what the process is) |
while you're at it it would be nice to make changing the value of deny-warnings not rebuild the compiler, could inject it in the rustc shim so cargo doesn't know about it (my understanding is @epage is working on upstream support for this in cargo, but in the meantime) |
I'm assuming @jyn514 is referring to rust-lang/cargo#8424. As an outsider, warnings failing during development are a major hindrance to experimenting and should be limited to CI. Yes, it only fails on CI but the user was generally warned CI will fail, no pun intended. This is how most projects I interact with operate, so its not unheard of precedence. |
this is what we talked about yesterday, remember - for large workspaces the warnings often get lost: rust-lang/cargo#8749 |
yea