Fair enough, and good points.
On Fri, 9 Sep 2016 12:50:29 +0200 Bastien Montagne <[email protected]> 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 <[email protected]> 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 <[email protected]> >>> 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 >>>> [email protected] >>>> https://lists.blender.org/mailman/listinfo/bf-python >>>> >>> >>> >> >> > >_______________________________________________ >Bf-python mailing list >[email protected] >https://lists.blender.org/mailman/listinfo/bf-python -- Regards, Christopher _______________________________________________ Bf-python mailing list [email protected] https://lists.blender.org/mailman/listinfo/bf-python
