logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <rem...@yahoo.com>
Subject Re: Final (again) (was Re: r1501104)
Date Wed, 10 Jul 2013 01:48:39 GMT
To be fair to Gary, he has been putting in a lot of effort to try to improve the code quality,
and this particular thing is something that was discussed and agreed on in the mailing list.
(I am agnostic on the use of final, btw.)

Ralph, I understand your annoyance at losing your changes (but it sounds like an IDE issue).
By the nature of Gary's changes it means making small fixes to many files.
I wasn't aware of any policy of giving warning before touching many files, but if that would
be useful we can adopt this as a group, I would not mind.

Nick, your comments on timing make sense, but personally I have trouble planning in advance
when I can spend time on Log4j. When I do finally have time I go ahead and clear up as many
things as I can from my to-do list. (And it is not like we have a QA team whose work would
be invalidated because we made changes after they finished testing.)

(And here is where I insert a joke that makes everyone relax again, but it seems like my correspondence
course in how to be funny is not paying dividends... I can't think of anything. I will ask
my money back!)

Remko

Sent from my iPhone

On 2013/07/10, at 10:13, Nick Williams <nicholas@nicholaswilliams.net> wrote:

> I see.
> 
> I imagine we could have this discussion every time a new developer comes on board. :-)
> 
> So, I see the points they're making, and I don't necessarily disagree with all of them
ipso facto. I, personally, always try to make instance variables final when appropriate. However,
I also think that each developer knows best the intent of his code, so if we want to put this
"always use final when possible" practice into place, the best way to do that is to train
developers to use it. Put it in the developer guidelines for contributors and committers,
etc. This blanket changing of dozens of files that the changer has no knowledge of and isn't
even reviewing individually concern me.
> 
> And that doesn't even address the fact that these changes increase the Checkstyle errors.
The additional of the six characters per variable (in a method with many parameters, that's
a lot of characters) causes some lines of code to exceed the 120 character limit and trigger
Checkstyle.
> 
> Finally, even /if/ I were to agree to blanket changes like this, I think they were done
at the wrong time. At $work, we have a policy about such changes: Blanket reformats and policy
changes are always made at the /beginning/ of a development cycle (immediately after a release)
as opposed to at the /end/ of a development cycle (immediately before a release, as GG has
done with MANY changes in the last few days). While such changes /shouldn't/ change program
logic, tools (Eclipse) are only as good as the people who write them (don't get me started
on my opinion of Eclipse) and can make mistakes. We shouldn't be doing things like this right
before a release.
> 
> My $0.02.
> 
> Nick
> 
> On Jul 9, 2013, at 7:14 PM, Ralph Goers wrote:
> 
>> This topic was covered in an email thread that started with this email - http://mail-archives.apache.org/mod_mbox/logging-log4j-dev/201301.mbox/%3CCACZkXPwsDyD-X%2B5wHuq4X26wzCZnsHJ%3Ds5rLJNKe71XYyg6LfA%40mail.gmail.com%3E.
 Unfortunately I don't know how to link the whole thread from the apache archives but you
can find it at http://marc.info/?t=134978818500007&r=1&w=2.
>> 
>> I happen to agree with you, as you will see from the thread.  I find it more annoying
that many, many files were changed at once with no warning.  I also suspect Gary does this
through something automated in Eclipse, which means he doesn't see the comments that say "don't
do that".
>> 
>> Ralph
>> 
>> On Jul 9, 2013, at 4:41 PM, Nick Williams wrote:
>> 
>>> Gary,
>>> 
>>> Earlier today you committed r1501104 with the following comment:
>>> 
>>>> Add final modifier to private fields.
>>>> Add final modifier to method parameters.
>>>> Add final modifier to local variables.
>>> 
>>> I've seen you do similar things before, too. Can you explain a little bit why
you do this? There's certainly no compiler value--the compiler for a long time has been capable
of determining whether a variable is ever changed and optimizing appropriately. All it seems
to do to me is clutter the code...significantly. It makes the code harder to read, IMO.
>>> 
>>> Can you justify the intentional mass change of this nature?
>>> 
>>> Thanks,
>>> 
>>> Nick
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> 

Mime
View raw message