[Bf-python] Handling cleanup patches from new contributor

[Christopher Barry]

Fair enough, and good points.


On Fri, 9 Sep 2016 12:50:29 +0200
Bastien Montagne <montagne29 at wanadoo.fr> wrote:

>I have two problems with those patches:
>
>I) I don’t really see the point here. Most of those changes are mere 
>matter of tastes, they do not give many (if any at all) speedup etc. 
>They are not bad in themselves, just don’t see the point on wasting
>time reviewing, applying, testing, handling possible issues (typos or
>worse, see below) afterwards, etc.
>
>II) Some of those changes are potentially dangerous if not *very* 
>carefully triple-checked. For example, replacing 'a = a + 1' by 'a +=
>1' is on first sight a mere matter of taste. *BUT* those two
>expressions are not the same thing, not with mutable types, see 
>http://stackoverflow.com/questions/2347265/why-does-behave-unexpectedly-on-lists/2347423#2347423 
>!
>
>We had a nice guy, knowing Blender codebase much better than pretty
>much any of us, who used to do large cleanups like that - how often
>did we had to fix some stupid typo, forgotten pieces of code, etc.
>afterwards - often even fixing broken build? And Blender is much, much
>more built and tested on daily basis than its addons. What I mean here
>is: cleanup *is not* a trivial activity, not even when you are an
>expert in both the language and the project.
>
>So my point is: leave cleanups to addons' authors/maintainers. We have
>a lot (too much) to do already with bug reports and patch reviews, 
>including in addon area… I’m sorry for the author of those patches,
>but I personally don’t feel like taking some time to go over them in
>detail, carefully checking they do not break/change behaviors
>unexpectedly, just for sake of some slightly better looking code.
>
>I don’t mind if someone takes time to do this - but I’d like to stress 
>this: please only consider changes on 'OFFICIAL' addons, the others we 
>should only limit ourselves to critical bugfixes, period.
>
>
>
>Le 08/09/2016 à 17:22, Christopher Barry a écrit :
>> Is it possible to merge the patches into a branch and ask on the list
>> for people familiar with the addons the patch addresses to try the
>> branch and test the changes?
>>
>> If all goes well, e.g. no complaints after a month or so, just merge
>> that into master.
>>
>> Getting people to actively investigate and cleanup other's code is
>> probably not easy to do. Don't alienate this angel :)
>>
>> -C
>>
>>
>> On Thu, 8 Sep 2016 17:03:48 +0200
>> Sergey Sharybin <sergey.vfx at gmail.com> wrote:
>>  
>>> Hi,
>>>
>>> There are some reasonable changes from (a) pep8 point of view (which
>>> we're trying to stick anyway) and (b) performance point of view.
>>> Rejecting such patches seems weird to me.
>>>
>>> Also, instead of rejecting patches and still asking maintainers to
>>> check on something is even more stupid. If we want to go this route,
>>> just ask the guy to do per-addon changes and assign those to
>>> maintainers.
>>>
>>> Or simply go ahead and apply reasonable patches globally on the
>>> whole scripts folder.
>>>
>>>
>>>
>>> On Thu, Sep 8, 2016 at 3:07 PM, Julian Eisel <eiseljulian at gmail.com>
>>> wrote:
>>>  
>>>> Hi all,
>>>>
>>>> Today a new contributor submitted a bunch of cleanup patches for
>>>> Add-ons (D2201, D2203, D2204, D2205, D2206, D2207, D2208, D2209,
>>>> D2210). This moves us in a kinda stupid situation: While we don't
>>>> want to disappoint a contributor (a new one, who probably put quite
>>>> some effort into the patches), we also don't want to spend much
>>>> time reviewing/testing biggish cleanup patches with basically zero
>>>> benefit, except of consistent code style (though I agree that this
>>>> has its importance too). Especially now that Campbell isn't
>>>> available to help anymore.
>>>> It's also our general policy to let Add-on maintainers handle fixes
>>>> and cleanups as much as possible.
>>>>
>>>> So as said, this is a kinda stupid situation and we're not sure how
>>>> to solve it. I'd propose we reject the patches, but use this
>>>> mailing list to ask Add-on maintainers to have a look at the
>>>> patches. They can then review and merge the changes that apply to
>>>> their Add-ons (with proper credits please).
>>>>
>>>> We discussed this briefly in #blenderpython and agreed on rejecting
>>>> the patches (after all, author could have contacted us earlier
>>>> too), but I decided to write this quick mail to avoid frustrated
>>>> contributors.
>>>>
>>>> Cheers,
>>>> - Julian -
>>>> _______________________________________________
>>>> Bf-python mailing list
>>>> Bf-python at blender.org
>>>> https://lists.blender.org/mailman/listinfo/bf-python
>>>>     
>>>
>>>  
>>
>>  
>
>_______________________________________________
>Bf-python mailing list
>Bf-python at blender.org
>https://lists.blender.org/mailman/listinfo/bf-python



-- 
Regards,
Christopher