[Bf-committers] Code cleanup woes

[Jason Wilkins]

Don't misunderstand.  The code I am "cleaning up" is already heavily
modified in other ways.

In general I tend to completely remove unrelated changes from any patch I make.

On Tue, Jan 29, 2013 at 7:12 PM, Campbell Barton <ideasman42 at gmail.com> wrote:
> @Jason, point taken.
>
> Its easy to point to code `improvements` that backfire then make
> statements like:
>  "any change to blender should have direct benefit to the user".
>
> Under this reasoning you wouldn't rename meaningless variables or add
> asserts to catch errors early on, split large functions up... etc.
>
>
>
> One thing you being up is doing cleanups in a branch...
>
> This is something I'd strongly discourage, it makes branch review
> really difficult because you have to identify cleanups from functional
> changes --- ask the dev is some change was intentional or not... just
> adds overhead to reviewing.
>
> Also if you track a bug back to a revision that has cleanup mixed with
> functional changes it takes longer to figure out the cause of the bug.
> And as you say, means you get many more conflicts in the branch.
>
> Id much prefer devs resist cleanup in branch and wait until after
> their work is merged into trunk.
>
> On Wed, Jan 30, 2013 at 11:18 AM, Jason Wilkins
> <jason.a.wilkins at gmail.com> wrote:
>> With all due respect, I did not miss the point.  Stephan Swaney got
>> what I meant.
>>
>> What frustrates me is my mistake is lumped in with this.  I did not
>> make my change solely because I thought the code was ugly.  I did it
>> because the code was out of sync with my branch and I missed a bugfix
>> because of it.  I screwed up and reintroduced the bug, but that
>> doesn't have anything to do with "code cleanup".  I thought the change
>> was more readable, so that is what I put in the change log.
>>
>> Anyway, I'm over it now.  The change will be made once the
>> compatibility stuff gets merged.  I just lost a chance to remove a
>> minor merge headache.
>>
>> The compatibility code is full of these kinds of cleanups.  That is
>> why I'm testing the hell out of it.
>>
>> On Sun, Jan 27, 2013 at 3:29 AM, Ton Roosendaal <ton at blender.org> wrote:
>>> Hi,
>>>
>>> You miss the point Jason. It's not about being stupid or not thinking enough.
>>> It shouldn't even have come up in a coders mind to "fix" it.
>>>
>>> -Ton-
>>>
>>> ------------------------------------------------------------------------
>>> Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
>>> Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands
>>>
>>> On 26 Jan, 2013, at 22:02, Jason Wilkins wrote:
>>>
>>>> I think this would not have broken if MEM_reallocN was exactly like
>>>> the standard library realloc.  The slight differences probably mislead
>>>> whoever was making this change.
>>>>
>>>> On Sat, Jan 26, 2013 at 6:42 AM, Ton Roosendaal <ton at blender.org> wrote:
>>>>> Hi all,
>>>>>
>>>>> Here's another nice illustration of code cleanup that caused bugs;
>>>>> http://projects.blender.org/scm/viewvc.php?view=rev&root=bf-blender&revision=54058
>>>>>
>>>>> The idea of "replace calloc + memcpy with recalloc" sounds nice, but it breaks the case when memcpy is of length zero. (zero hairs, use 'add' brush in particle mode).
>>>>>
>>>>> May I quote Joelonsoftware here?
>>>>>
>>>>> "Old code has been used. It has been tested. Lots of bugs have been found, and they've been fixed. There's nothing wrong with it. It doesn't acquire bugs just by sitting around on your hard drive. Au contraire, baby!"
>>>>>
>>>>> Great fun read:
>>>>> http://www.joelonsoftware.com/articles/fog0000000069.html
>>>>>
>>>>> Of course we should never be afraid to recode parts of Blender, but it shouldn't be with the naive assumption that the old bad messy code wasn't functional. New code should be totally and undeniably better, for users. Not for coders. :)
>>>>>
>>>>> We can moan and complain about our current bad particle code a lot, but it has offered a lot of good for Blender for many years. The challenge is not to make a new system with clean design, but to make a new system that's far superior.
>>>>>
>>>>> -Ton-
>>>>>
>>>>> ------------------------------------------------------------------------
>>>>> Ton Roosendaal  Blender Foundation   ton at blender.org    www.blender.org
>>>>> Blender Institute   Entrepotdok 57A  1018AD Amsterdam   The Netherlands
>>>>>
>>>>> _______________________________________________
>>>>> Bf-committers mailing list
>>>>> Bf-committers at blender.org
>>>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>>> _______________________________________________
>>>> Bf-committers mailing list
>>>> Bf-committers at blender.org
>>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>>
>>> _______________________________________________
>>> Bf-committers mailing list
>>> Bf-committers at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>> _______________________________________________
>> Bf-committers mailing list
>> Bf-committers at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>
>
>
> --
> - Campbell
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers