Updated wiki docs based on feedback. http://wiki.blender.org/index.php/Dev:Doc/New_Committer_Info#Commit_.28Best_Practice.29 (points 2, 3)
http://wiki.blender.org/index.php/Dev:Doc/Code_Style#Trailing_Space - Added note to avoid tailing space for new code. AFAICS this is just putting in writing whats been happening in Blender dev anyway, the recent add/remove/add of whitespace just prompted noting in the wiki. On Sun, Dec 28, 2014 at 8:27 AM, Sergey Sharybin <[email protected]> wrote: > So for the best practice section i guess we should add: > > - Avoid modifying whitespace (indentation, trailing whitespace) for code > your commit doesn't change > - Avoid having trailing whitespace in the new code > - Don't do minor cleanup to existing code (such as trailing whitespace > change) (note: original statement from you was telling about "minimal > changes" which is really vague, ie. https://developer.blender.org/rBb11a2f7 > is kinda minimal but still kinda useful) > > I'm also not sure about commits like > https://developer.blender.org/rBee36e75b855ca32c794ef2d1b2be7fe5d7067f55 In > theory we can go berzerker and fix GPL headers all over the blender as well > (with all that legacy original thing) but would vote for rather not do > this. So while we're on a guidelines wave, shall we add some notes in the > guide about this? Guess for this we can phase 3d as: > > - Don't do minor cleanup to existing code (such as trailing whitespace > change, capitals change, sentence finish dot change) > > P.S. Well, granted the license header commit was done by the miaintainer > but i;d really suggest not making it a practice for upgrade headers all > over the place.. > > On Sun, Dec 28, 2014 at 2:02 AM, Campbell Barton <[email protected]> > wrote: > >> On Sun, Dec 28, 2014 at 7:37 AM, Sergey Sharybin <[email protected]> >> wrote: >> > While i agree such changes are real PITA the proposed statement in wiki >> is >> > too vague IMO. >> >> Yep, this could be nailed down better. (I wanted to avoid writing a >> lot of details but, seems being brief here isnt helpful!) >> >> > Is indentation fix a minor or not? Is brace placement changes minor or >> not? >> > This is a bit unclear and needs more clear statement (since i know one >> guy >> > who does all sort of cleanup all over the code :) >> >> The code-style stuff is generally there because it helps >> consistancy/readability. >> However there are a bunch of things we didnt add to the code-style >> doc, because its not really helping reading the code. >> >> Eg: >> - how to group includes. >> - number of blank lines between functions. >> - correct punctuation in comments (scentances ending with fullstops... >> etc). >> >> > Also, shall we have some general guide stating that trailing whitespace >> is >> > not recommended? Or maybe at least make it not recommended to add >> trailing >> > whitespace to code which isn't changed (so patches are easier to follow)? >> >> Agree, new code or code you rewrite, I rather not have trailing space, >> Some of my code has been edited by others and now has 5+ trailing >> tabs, dont care that much but rather avoid. >> git also shows trailing whitespace *red* in diff's :) >> >> But still prefer no bulk-whitespace commits. >> >> > On Sun, Dec 28, 2014 at 1:21 AM, Campbell Barton <[email protected]> >> > wrote: >> > >> >> Regarding devs adding/removing trailing whitespace. >> >> >> >> - >> https://developer.blender.org/rB0fc0cb351b5e838dce7a14e8fe68fb6f7125eab0 >> >> - >> https://developer.blender.org/rB5d8a207b67aee18cadec742246b9dc9c052bd344 >> >> - >> https://developer.blender.org/rBfd4720204399e16032574159699d5a26a250e6d5 >> >> - >> https://developer.blender.org/rB49052c61f8326c11bc733a040372481ea1d7438b >> >> >> >> These makes merging branches & applying patches a hassle, while giving >> >> minimal benifit, not even helping readability. >> >> >> >> I recall this topic came up before but couldn't find it on the wiki >> >> anywhere, so added a note: >> >> >> >> >> >> >> http://wiki.blender.org/index.php/Dev:Doc/New_Committer_Info#Commit_.28Best_Practice.29 >> >> >> >> ---- >> >> Don't make minor changes such as trailing whitespace to existing code >> >> (unless you're the module owner). >> >> ---- >> >> >> >> ... though this could be changed if other devs have a strong opinion. >> >> >> >> -- >> >> - Campbell >> >> _______________________________________________ >> >> Bf-committers mailing list >> >> [email protected] >> >> http://lists.blender.org/mailman/listinfo/bf-committers >> >> >> > >> > >> > >> > -- >> > With best regards, Sergey Sharybin >> > _______________________________________________ >> > 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 >> > > > > -- > With best regards, Sergey Sharybin > _______________________________________________ > 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
