Robert, You're right, this stuff is not clearly laid out anywhere, and I'm going to try to take on the task of fixing that (*after* documentation migration) if no one else is yet (I know the core guys are already aware of that issue and some work has already been done). It can be very frustrating when you're trying to contribute something to have stylistic stuff thrown up when the closest thing we have to a published style guide is four bullet points on the prototypejs.org/ contribute page. :-) And the medium (lighthouse) encouraging terseness doesn't help on the encouragement front.
It _is_ important for newly contributed code to fit in (which has previously been a terrible mish-mash and which core are trying to standardize, with big gains in the 1.6.1 stuff), as I'm sure you already know. My read of the ticket and patch is that these are the outstanding requests: 1. Use argLength and origArgLength or similar descriptive argument names rather than len0, etc. 2. Include a // comment in the implementation describing the why and how of your cool idea. (// and PDoc comments are stripped by rake dist, /* ... */ are not, hence making it a // comment -- it's for other developers). Both of those are things that have not been done enough in the past and have held back progress, hence trying to move move toward them. As a Prototype user, thank you again for your efforts at improving efficiency, greatly appreciated, -- T.J. Crowder tj / crowder software / com www.crowdersoftware.com On Sep 5, 10:19 pm, Robert Kieffer <bro...@gmail.com> wrote: > Whups, sorry about butchering your name. I'm seriously sleep deprived > these days - a result of our new 5-month old kid. :-) > > Re: Documentation for how and why the patch works - Is that a new > requirement for Prototype code? This is the first I've heard of > something like this. My original post in the discussion (link above) > goes into a fair bit of detail. Where does this documentation go? > What is expected? Sorry, it's been a while since I looked at the > prototype code base and, as of March, I thought my patch was pretty > consistent with how things are done. > > I apologize for being so obtuse on this, but can you please elaborate > on what you mean by "a final, reviewed, and cleaned up patch"?In re- > reading the ticket, I simply don't see where the issues are. The only > issue that I guess didn't get addressed was your request for more > descriptive variable names... but that didn't seem to be a showstopper > (nor consistently applied w/in the existing codebase). But if it > makes a difference, I'm happy to do the renaming. > > Cheers, > - rwk > > On Sep 5, 12:11 pm, Tobie Langel <tobie.lan...@gmail.com> wrote: > > > > > Hi again, Robert. > > > The patch linked by kangax certainly doesn't account for the various > > things we discussed back then. > > > It notably doesn't document the reasons why and how your (very smart) > > implementation works. > > > That patch also has various "stylistic" issues which I remember > > discussing and that we had all agreed to modify for a final patch. > > > If I recall correctly, this patch just didn't make it in because a > > final, reviewed and cleaned up patch wasn't submitted. An unfortunate > > yet frequent issue with OSS. > > > Given the amount of work that was put in this patch and the huge perf > > benefits it brings about, I think it makes sense to add it to an > > otherwise frozen 1.6.1 version provided a proper patch gets submitted. > > > Best, > > > Tobie > > > Unrelated P.S.: Would appreciated not seeing my first name > > butchered. :-) --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to prototype-core-unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~----------~----~----~----~------~----~------~--~---
