Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@bamarsha
Copy link
Contributor

@bamarsha bamarsha commented Jan 20, 2021

Updated the CI to build with /property:TreatWarningsAsErrors=true. This means that warnings are allowed when developing locally, but all warnings must be fixed or suppressed before a PR can be merged.

This applies to C# and F# warnings, but not MSBuild and Q# warnings.

@bamarsha bamarsha changed the title Treat warnings as errors Treat C# and F# warnings as errors Jan 20, 2021
@bamarsha bamarsha marked this pull request as ready for review January 20, 2021 01:39
@bamarsha bamarsha requested a review from swernli January 22, 2021 18:03
@swernli
Copy link
Contributor

swernli commented Jan 22, 2021

How will this affect things like classes being marked as deprecated? I know we generally try to not to use deprecated structures, but sometimes during multi-part work it's convenient to add the new infrastructure, mark the old one as deprecated, and then go around update all the consumers.

EDIT: never mind, I realize now that we can just put specific suppressions in if there needs to be that kind of delay between deprecation and update :)

@bamarsha
Copy link
Contributor Author

Yeah, you can use #pragma warning disable (C#) or #nowarn (F#) to suppress warnings that are false positives or warnings that will be fixed later. So this PR just requires that every warning is looked at and is either fixed or marked as OK.

Copy link
Contributor

@cesarzc cesarzc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this Sarah!

@bamarsha bamarsha merged commit b6c6912 into main Jan 23, 2021
@bamarsha bamarsha deleted the samarsha/warnings-as-errors branch January 23, 2021 07:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants