Review build warnings #61

Closed
thefiddler opened this Issue Feb 17, 2014 · 3 comments

Comments

Projects
None yet
5 participants
Contributor

thefiddler commented Feb 17, 2014

A typical build on a modern version of Mono/.Net results in roughly 160 warnings. We should review these warnings and either fix them or silence them when harmless (e.g. unused private field warning for reserved fields in interop structures.)

(Originally accidentally posted in a pull request related to this) In addition to normal warnings I would like to ask if going over the code and look at warnings given from utilities like ReSharper could be included in this. Perhaps including warnings on code style standards?
n798tnj

Contributor

thefiddler commented Feb 25, 2014

Static analysis is really useful, but it takes some effort to distinguish useful warnings from fluff. When I last tried this on OpenTK, I got several thousands of warnings but only a few dozen of those were actual code quality issues.

For example, in the attached screenshot it is suggested to move Lindgren.Network/Encryption to Lindgren/Network/Encryption and rename private variables from m_* to _*. These are cosmetic issues. Fixing these would make other forks more difficult to merge and potentially introduce new bugs, for a very modest improvement in code quality.

I would happily accept PRs that fix potential issues revealed through static analysis, but I am quite skeptical about the value of fixing cosmetic issues in private code.

Frassle added a commit to Frassle/opentk that referenced this issue Nov 3, 2014

Add XML documentation to matrix Add methods.
Fixes warnings [#61] about missing documentation on public members.

Frassle added a commit to Frassle/opentk that referenced this issue Nov 3, 2014

Remove unused field.
Fixes warnings [#61] by removing an unused field.

Frassle added a commit to Frassle/opentk that referenced this issue Nov 3, 2014

Remove unused fields/variables.
Fixes warnings [#61] by removing some unused variables and an unused field.
Field was private so no inheritance concerns.

Frassle added a commit to Frassle/opentk that referenced this issue Nov 3, 2014

Add #pragma warning disable for unused field warnings.
Fixes warnings [#61] by disabling unused field warnings for two structures in
HidProtocol. These fields aren't currently used by OpenTK but the stuctures are
used in native marshalling so must match the documented structures perfectly.

Frassle added a commit that referenced this issue Nov 3, 2014

@daerogami daerogami self-assigned this Mar 22, 2017

Contributor

Nihlus commented Jun 8, 2017

@varon @daerogami There is practically nothing left of this issue, and it should probably be closed. Smaller, more quantifiable issues can be opened for any warnings still present.

@varon varon closed this Jun 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment