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 <[email protected]> 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 > <[email protected]> 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 <[email protected]> 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 [email protected] 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 <[email protected]> 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 [email protected] www.blender.org >>>>> Blender Institute Entrepotdok 57A 1018AD Amsterdam The Netherlands >>>>> >>>>> _______________________________________________ >>>>> Bf-committers mailing list >>>>> [email protected] >>>>> http://lists.blender.org/mailman/listinfo/bf-committers >>>> _______________________________________________ >>>> Bf-committers mailing list >>>> [email protected] >>>> http://lists.blender.org/mailman/listinfo/bf-committers >>> >>> _______________________________________________ >>> Bf-committers mailing list >>> [email protected] >>> http://lists.blender.org/mailman/listinfo/bf-committers >> _______________________________________________ >> Bf-committers mailing list >> [email protected] >> http://lists.blender.org/mailman/listinfo/bf-committers > > > > -- > - Campbell > _______________________________________________ > Bf-committers mailing list > [email protected] > http://lists.blender.org/mailman/listinfo/bf-committers _______________________________________________ Bf-committers mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-committers
