Treat warnings as errors

Oct 16, 2011 at 10:51 PM

I'm not sure if it was a Mono update or a change to the DotSpatial project configuration, but now when I compile the solution in Mono it treats warnings as errors. Each individual project has this property turned on. Is this intentional?

If so, that's great, except there are a lot of needless warnings being thrown right now. I'll be happy to clean them up, but I need to know if this is what you intended.

Oct 17, 2011 at 7:51 PM

After the group discussion, I modified each DotSpatial project configuration so that it treats warnings as errors on Oct 7 (please check Change Set 5dae556ce185 on the Source Code Tab). By doing that, every DotSpatial programmer can be aware of the warnings he or she creates when coding, which save other programmers' time and energy to fix the warning.

Oct 17, 2011 at 8:03 PM

That is a great idea, except when code strewn with warnings gets committed to the main repo. If you have code that throws warnings, and you cannot change the code, wrap it in a #pragma warning disable <err_code>.

Oct 17, 2011 at 8:06 PM

The code in the main repo should always compile. Mercurial is designed to make this easier to accomplish. Commit broken code to your own local repo, but don't push it to the main repo until it is clean. Otherwise others will have to spend time fixing your errors, and they can't get their own work done. That was my weekend in a nutshell. There are at least a dozen files that currently fail to compile.

Coordinator
Oct 18, 2011 at 5:14 PM

This is a good point, Carlos. Perhaps we need to Undo that change set so that the warnings are NOT treated as errors for now - at least not in the version that is committed to the main repository. This way, as Carlos points out, the code in the main repo will always compile.

Developer
Oct 18, 2011 at 6:19 PM
There is a way to put a global set of ignore warning in the project.

On Tue, Oct 18, 2011 at 10:14 AM, danames <notifications@codeplex.com> wrote:
> From: danames
>
> This is a good point, Carlos. Perhaps we need to Undo that change set so
> that the warnings are NOT treated as errors for now - at least not in the
> version that is committed to the main repository. This way, as Carlos points
> out, the code in the main repo will always compile.
>
> Read the full discussion online.
>
> To add a post to this discussion, reply to this email
> ([email removed])
>
> To start a new discussion for this project, email
> [email removed]
>
> You are receiving this email because you subscribed to this discussion on
> CodePlex. You can unsubscribe or change your settings on codePlex.com.
>
> Please note: Images and attachments will be removed from emails. Any posts
> to this discussion will also be available online at codeplex.com
Developer
Oct 19, 2011 at 1:59 AM

The code in the repository does compile (as shown on the build server). It seems Carlos is using a different setup for building (i.e. mono) and it's not working there.

Oct 20, 2011 at 9:22 PM

Visual Studio ignores some things that Mono catches. For instance, if you declare a variable and never use it, VS doesn't signal a warning, but Mono does. Same thing if you call an obfuscated method. We will have to find a way to make VS report all warnings, not just some of them. Or, if that isn't possible, the "treat warnings as errors" setting simply will not do. I cannot make any headway on my work because I have to keep cleaning up "errors" that you guys are not even aware of.

Developer
Oct 21, 2011 at 3:13 PM

How easy would it be to change the behavior in your own development environment so that it does not treat warnings as errors or so it ignores the same warnings that VS ignores?

Would that change make it difficult to stay in sync with the repository?

Outside DotSpatial I prefer to leave warnings as warnings. They are still easy to see and I typically fix them before I commit, but they do not prevent compiling and running. Occasionally I will purposely leave a line of code that causes a warning as a reminder to myself that I need to return to that section of code.

By the way, VS 2010 does warn when a variable is declared but not used:

"The variable 'x' is declared but never used"

The warning changes if the declaration also assigns a value:

"The variable 'x' is assigned but its value is never used"

Perhaps someone has turned off these warnings in VS if they are not seeing them.

Developer
Oct 21, 2011 at 4:13 PM
Other option,
enforce warnings as errors on the build server.

On Fri, Oct 21, 2011 at 8:13 AM, [email removed] wrote:
> From: vatavian
>
> How easy would it be to change the behavior in your own development
> environment so that it does not treat warnings as errors or so it ignores
> the same warnings that VS ignores?
>
> Would that change make it difficult to stay in sync with the repository?
>
> Outside DotSpatial I prefer to leave warnings as warnings. They are still
> easy to see and I typically fix them before I commit, but they do not
> prevent compiling and running. Occasionally I will purposely leave a line of
> code that causes a warning as a reminder to myself that I need to return to
> that section of code.
>
> By the way, VS 2010 does warn when a variable is declared but not used:
>
> "The variable 'x' is declared but never used"
>
> The warning changes if the declaration also assigns a value:
>
> "The variable 'x' is assigned but its value is never used"
>
> Perhaps someone has turned off these warnings in VS if they are not seeing
> them.
>
> Read the full discussion online.
>
> To add a post to this discussion, reply to this email
> ([email removed])
>
> To start a new discussion for this project, email
> [email removed]
>
> You are receiving this email because you subscribed to this discussion on
> CodePlex. You can unsubscribe or change your settings on codePlex.com.
>
> Please note: Images and attachments will be removed from emails. Any posts
> to this discussion will also be available online at codeplex.com
Oct 21, 2011 at 5:01 PM

The "treat warnings as errors" setting occurs at the project level, not the host environment level. My settings in Mono are identical to the ones you see in VS because they are contained in the project files, in mercurial, in the solution.

I have over two dozen examples in the DotSpatial project of unused variable warnings that are ignored by the VS compiler. Matthew saw this for himself yesterday when we were on the phone. The question is: is there some way to make VS see these warnings? That is the pertinent question right now.

Enforcing warnings as errors on the build server will be fruitless if it's a Windows server running VS. I intend to create some sort of automated build process on a Linux machine. These errors you cannot see need to be reported to you in some way.

Developer
Oct 21, 2011 at 5:41 PM

Take a peek at http://dotspatial.codeplex.com/SourceControl/changeset/changes/bd4dced326d4#DotSpatial.Positioning%2fDotSpatial.Positioning%2fNmeaInterpreter.cs to see what is meant by "Visual Studio 2010 appears to be missing warnings."

Developer
Oct 21, 2011 at 6:00 PM
Sounds more like the group wants stylecop enforcement of the rule.
http://stackoverflow.com/questions/6370278/how-to-use-stylecop-with-teamcity

In the build server, you can pass in properties:
in the team city command line parameter field
/property:WarningLevel=1
generates:
[10:58:24]: [Csc]
C:\Windows\Microsoft.NET\Framework\v4.0.30319\Csc.exe /noconfig
/nowarn:1701,1702 /nostdlib+ /errorreport:prompt /warn:1 /define:TRACE
....

This should be warnings as errors:
/property:WarningLevel=4

and maybe these elements:
<CodeAnalysisIgnoreBuiltInRules>false</CodeAnalysisIgnoreBuiltInRules>
<CodeAnalysisFailOnMissingRules>false</CodeAnalysisFailOnMissingRules>

On Fri, Oct 21, 2011 at 10:41 AM, mudnug <notifications@codeplex.com> wrote:
> From: mudnug
>
> Take a peek
> at http://dotspatial.codeplex.com/SourceControl/changeset/changes/bd4dced326d4#DotSpatial.Positioning%2fDotSpatial.Positioning%2fNmeaInterpreter.cs to
> see what is meant by "Visual Studio 2010 appears to be missing warnings."
>
> Read the full discussion online.
>
> To add a post to this discussion, reply to this email
> ([email removed])
>
> To start a new discussion for this project, email
> [email removed]
>
> You are receiving this email because you subscribed to this discussion on
> CodePlex. You can unsubscribe or change your settings on codePlex.com.
>
> Please note: Images and attachments will be removed from emails. Any posts
> to this discussion will also be available online at codeplex.com
Developer
Oct 21, 2011 at 6:13 PM
this one:
/property:TreatWarningsAsErrors=true

http://msdn.microsoft.com/en-us/library/z7fxbe10.aspx
On Fri, Oct 21, 2011 at 11:00 AM, david valentine
<david.valentine@gmail.com> wrote:
> Sounds more like the group wants stylecop enforcement of the rule.
> http://stackoverflow.com/questions/6370278/how-to-use-stylecop-with-teamcity
>
> In the build server, you can pass in properties:
> in the team city command line parameter field
> /property:WarningLevel=1
> generates:
> [10:58:24]: [Csc]
> C:\Windows\Microsoft.NET\Framework\v4.0.30319\Csc.exe /noconfig
> /nowarn:1701,1702 /nostdlib+ /errorreport:prompt /warn:1 /define:TRACE
> ....
>
> This should be warnings as errors:
> /property:WarningLevel=4
>
> and maybe these elements:
>  <CodeAnalysisIgnoreBuiltInRules>false</CodeAnalysisIgnoreBuiltInRules>
>    <CodeAnalysisFailOnMissingRules>false</CodeAnalysisFailOnMissingRules>
>
> On Fri, Oct 21, 2011 at 10:41 AM, mudnug <notifications@codeplex.com> wrote:
>> From: mudnug
>>
>> Take a peek
>> at http://dotspatial.codeplex.com/SourceControl/changeset/changes/bd4dced326d4#DotSpatial.Positioning%2fDotSpatial.Positioning%2fNmeaInterpreter.cs to
>> see what is meant by "Visual Studio 2010 appears to be missing warnings."
>>
>> Read the full discussion online.
>>
>> To add a post to this discussion, reply to this email
>> ([email removed])
>>
>> To start a new discussion for this project, email
>> [email removed]
>>
>> You are receiving this email because you subscribed to this discussion on
>> CodePlex. You can unsubscribe or change your settings on codePlex.com.
>>
>> Please note: Images and attachments will be removed from emails. Any posts
>> to this discussion will also be available online at codeplex.com
>
Oct 22, 2011 at 7:42 PM

I have an automated clean and build script running. If you wish to receive the email from this script, please let me know. Dan and Matthew are on the list by default. I think you'll find the report illuminating.

If a module cannot compile because of errors, none of the modules that depend on it will even attempt to compile. Therefore the report does not show all errors, only the "1st tier" errors. Once those are fixed, the next layer of errors will be exposed, and so on.

I will set up the script to run once a day. Provide feedback if you want to see it run more often. It would probably be helpful, especially at first.

Developer
Oct 25, 2011 at 8:55 PM

I noticed that it is a known issue that the c# compiler does not create warnings for unused variables and fields (http://connect.microsoft.com/VisualStudio/feedback/details/362970/c-compiler-does-not-create-warnings-for-unused-variables-and-fields)

We're going to potentially have a number of warnings appear when we run FxCop on the project. If going that route, we are going to need to make it easy to let developers known when their code creates FxCop warnings, and we are going to have to expend an initial effort to reduce the FxCop warning count. The advantage of using code analysis is, of course, more maintainable code.

Oct 25, 2011 at 9:25 PM

Absolutely agreed. The fact that there are certain warnings that the Microsoft C# compiler does not report is the fundamental problem that we need to address. Replace "FXCop" with "Mono build cron job", and you have the solution I have in place, running, right now. Just give me your email address and I will add you to the report. It's that easy. Email me at ckonstanski at pippiandcarlos.com.

Developer
Oct 25, 2011 at 10:43 PM

I noticed that we might expect a large volume of errors: http://dotspatial.codeplex.com/wikipage?title=FXCopNonBreakingErrors

 

I've backed out 5dae556ce185 as will we need to pursue another solution other than /property:TreatWarningsAsErrors=true.