[Opensim-dev] Warning Safari (was Re: Current Status of OpenSim Git Master)

[Melanie IMAP]

Hi Tom,

I gave you the OK for a warning safari but I believed you understood that this is alpha code.

Of course we NEED the commented code and commented code does NOT generate warnings.

We don't need a "code cleanup", projects in the development state we are in would lose too much.

I would like for you to continue, but please take these things to heart:

- Unused variables, including those assigned to but not used, can only be commented, not removed. In many cases you will find that the values ganerated are used in debugging statements further down that have been commented!

- Commented code needs to be left where it is. Also,  a warning safari is not a mandate to beautify code. No reordering and no whitespace corrections should be done.

Limited code cleanup or whitespace correction can be done outside the warning safari and after it's complete.

The issue here is that whitespace changes break pulling from git for downstream projects needlessly. So does code reordering, etc.

Regarding the team, we are a multinational team and not all of us have English as their first language or America as their cultural origin. Please do feel free to approach people about perceived slights, but let's not make a big deal out of a wording. What counts is the spirit, not the letter.

- Melanie

On 21 Nov 2015, at 10:14, Tom Frost <tomfrost at linkwater.org> wrote:

Hi opensim devs,

> On Fri, Nov 20, 2015 at 11:50:09PM +0100, Tom Frost wrote:
>> On Fri, Nov 20, 2015 at 02:33:53PM +0100, Melanie wrote:
>> a warning safari is highly appreciated. You don't need to create a
>> Mantis for each warning, simply create one for the warnings and
>> attach the patches to it. For pure warning removal with no
>> functional changes, the patches can be larger than what we usually
>> ask for with functional changes.
> 
> Alright, i've gone ahead and created this issue:
> 
> http://opensimulator.org/mantis/view.php?id=7746
> 
> and attached 13 patches to address a first batch of low-hanging 
> fruit. I thought it would be convenient for picking which ones
> to apply.. But I can resubmit it as one large path if that
> is preferred. Just let me know!

---+ Unused variables and warnings

I got this response:

'Hi, thanks but please stop for now :( You are also removing comments, dead code,
etc, and that needs to be checked case by case, even some unused variables are
there waiting future use You removed for example debug messages that we put back every time we
need.  I can't apply patches like this automatically. I will use them as
reference.  I did remove a few of those warnings, increasing the total reported by 2
;)´

Unused variables that are there waiting for future use and that
generate warnings are in violation with the coding standards as stated
on http://opensimulator.org/wiki/Coding_standards:

"Warnings

   Please keep the code warning free, using #pragma only if absolutely
necessary. For instance ..."

Are all the core devs aligned in the acceptance of the practice of
leaving in unused variables that generate warnings during compilation
because someone at some point may want to use them?

If so, please rewrite the coding standards, so that new people like me
don't waste their time.

---+ Debug statements and commented-out code

I'm ok with redoing the patches to leave in large fragments of
commented-out code and stray debug log statements that have been
commented out (if indeed all core devs are fine with the ugliness this
introduces into the codebase).

As for debug statements, wouldn't it be better to define them as such by
not commenting them out but leaving them in and make them conditional on
a variable that configures the debug level of logging?

---+ Bad attitude

As a sidenote, telling someone to 'please stop for now :(' is not very
productive. If I need to do things differently just say so, instead of
using discouraging wording. From the comment above it feels like my work
is not appreciated and i should not attempt to help out.



Please let me know how to proceed.

Thanks,

Tom
_______________________________________________
Opensim-dev mailing list
Opensim-dev at opensimulator.org
http://opensimulator.org/cgi-bin/mailman/listinfo/opensim-dev