On 2/9/07, Colin Mollenhour <[EMAIL PROTECTED]> wrote: > > > I think this patch has become quite refined and mature by now, I'd love to > get some more feedback from the Core guys... >
First of all, for all you joining late to the event discussion from several threads back; there's an Event patch [1] sitting in Trac for some time now that handles a lot. This and Colin's patch [2] don't overlap in functionality as much as in code; some clever merging will be required to take the best out of both patches. We had internal discussion about creating an event SVN branch in which these changes would be made - it may happen soon, depending on the people in charge of the repository. Colin, we still haven't discussed having your featureset in core, but that doesn't mean it won't happen. Currently, your patch doesn't feel exactly "Prototypish". First of all, I don't like splitting what we have into 2 things -- a namespace and a class -- when one can reside in another (eg. an event related class should reside in the Event namespace). Second, I don't like breaking chaining of observe(). Third, it has too much "private" properties (the underscore ones) and I can't stop thinking this featureset should take way *less* code. This doesn't mean I want you to "fix" your patch. No, it should stay the way it is now, but the ticket could stand tests. No, not unit tests (we're still figuring out how to unit-test events), but *functional* tests (the next best thing). Let the HTML document in [1] serve as the base for your tests. Keep em simple; there doesn't have to be much, just showing off the functionality and that it really works. -- [1] http://dev.rubyonrails.org/ticket/5786#comment:9 [2] http://dev.rubyonrails.org/ticket/7435#comment:1 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~----------~----~----~----~------~----~------~--~---
