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

Move warnaserror from build scripts to common props #389

Closed
weshaggard opened this issue Jul 27, 2018 · 5 comments
Closed

Move warnaserror from build scripts to common props #389

weshaggard opened this issue Jul 27, 2018 · 5 comments

Comments

@weshaggard
Copy link
Member

In build.ps1/build.sh we currently hard-code the msbuild /warnaserror option to the msbuild invoke commands. In general I like to make warnings as error but there are cases where someone might need to opt-out of that for a given project so I think we should switch to using the property MSBuildTreatWarningsAsErrors in our common props instead of the command line switch. That way individual projects can opt-out of the setting where as they cannot do that with the command line switch (at least I didn't see a way to do that).

dotnet/msbuild#1928 for reference about the warn as error feature for msbuild.

@tmat what are your thoughts?

@tmat
Copy link
Member

tmat commented Jul 27, 2018

I'd rather not do this a-priory. Let's wait until someone asks for it and have compelling reason.

@weshaggard
Copy link
Member Author

The reason I filed this is because I'm running up against it in corefx right now. There are some warnings that are now errors that are not so easy to fix and I'm trying to avoid too many distractions so I wanted to turn this off for now. I think having the knob will help folks onboarding as well as folks that that run up against an issue.

@tmat
Copy link
Member

tmat commented Jul 27, 2018

OK, Sounds reasonable, although if there is no forcing function these warnings will never get fixed ;)

@weshaggard
Copy link
Member Author

although if there is no forcing function these warnings will never get fixed

I agree we need this forcing function and if I do disable it I will ensure there is an issue tracking it.

Given the build scripts are checked in the repo I can also temporarily disable this flag via some option passed to the build script

@markwilkie
Copy link
Member

@wes took a different approach and this issue can be closed.

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

No branches or pull requests

3 participants