search

dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

Fail builds on warnings, and package updates #250

Closed AArnott closed 8 years ago

AArnott commented 8 years ago

Set appveyor builds to fail for any build warning. This should make it readily apparent when PRs should not yet be merged since we are and wish to remain warning clean.

Also update StyleCop.Analyzers, and NB.GV

AArnott commented 8 years ago

Do you support this policy, @vbfox ?

vbfox commented 8 years ago

:+1: x 100

Yes completly supported, we have that at work (Including StyleCop) and it's really good.

(Technically we don't use the catch-all treat warning as error due to csc emmiting non-ignorable warnings in some unavoidable cases so we list them, but the problem won't be present here I think)

We even had it enabled in Visual Studio to avoid anyone to commit bad code and notice only on CI build. (We've disabled it due to the current VS2016 OutOfMemory problems but plan to re enable it if Update 3 change that)

AArnott commented 8 years ago

Technically we don't use the catch-all treat warning as error due to csc emmiting non-ignorable warnings in some unavoidable cases so we list them, but the problem won't be present here I think

Yes, I have similar issues on some of my other projects. But since we're clean for PInvoke, we can use this to stay clean as long as possible. Maybe someday we'll have to dial it back. We'll see.

We even had it enabled in Visual Studio to avoid anyone to commit bad code and notice only on CI build.

I like moving discovery forward like that, but I don't like how that can really hamper inner-loop development. (e.g. "Why do I have to be stylecop clean while I'm just testing out an idea?")

vbfox commented 8 years ago

"Why do I have to be stylecop clean while I'm just testing out an idea?"

Yes we had this problem multiple times, our temporary solution was to maintain 2 rules files, the Debug build having warnings were the release one had errors. But nothing perfect there.