[Dojo-interest] On patches and enhancements (was Re: (Unofficial) FilteringTable enhancements)

[Tom Trenka]

Sasha Firsov wrote:
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> I understand and totally support heavier way to commit patches.
> Still, the statement will be less useful without support for
> architectural and code review process, which in fact was away from
> community. At least I seen it in many cases.
> 

No offense, but patch review will always be away from the community and
reserved for the core contributors and modules.  We've already had a few
instances where even contributors have "gone cowboy" on the codebase and
caused major issues with parts of the library that were hardly considered
beforehand.


Sasha Firsov wrote:
> 
> ...Anyway, runtime patch on existing widget class and/or object has reason
> to exist. You can say, it is not good for many reasons, but if it has a
> sense to many people, why not allow it to exist?
> 

It depends on the patch, what it does, how heavy it is in terms of
performance, whether or not the patch steps on another part of the code in a
major way, and whether or not it's truly useful to the community as a whole. 
All patches are considered (as long as there's a verifiable CLA attached to
it).  All I'm saying is that to date we've been very permissive about
accepting and incorporating patches--and going forward, we are going to be a
lot more strict about what gets into the actual core.


Sasha Firsov wrote:
> 
> So far extend and mixin are elegant in some way, but heavy, when runtime
> patch is simple and straight. And direct way to simulate core patches.
> If I will have a choice to either use custom extension widget or patch,
> the choice is obvious: runtime patch. To support multiple
> features(patches) using mixing/extend bunch of code need to be written.
> But for runtime patches you just need one JS per patch.
> 

You can always include a single JS file as well.  Really, it depends on what
your own preference is; if you want to have a single patch file, feel free. 
I suggested extend and mixin because both work well for what is done.  You
can also always use the traditional JS method:

someConstructorToApply.call(objectToApplyItTo);

...which is not nearly as heavy as you might think.  Of course, that comes
with it's own quirks...


Sasha Firsov wrote:
> 
> Architecturally correct way will be in supporting multiple dojoType
> statements, similar to multiple styles in class attribute. But multiple
> widgets on same tag (mixins?), I guess, is out of scope for now ...
> So still stick to "runtime patch" as the best way to apply few features
> to existing widget. Someone with me?
> 

You are now talking specifically about the widget system and not about Dojo
itself, and I can say for sure that there are major changes coming here as
well.  As far as supporting multiple dojoType attributes (or multiple values
within a dojoType attribute), its a neat idea but one thing we (core Dojo
committers) are learning is that the majority of performance problems--as
well as the high learning curve--of Dojo is in good portion due to the way
we tend to support multiple, highly complex ways of doing things.  This is
also something we are going to change--knowing full well that there are
people like you out there who can, if they really want it, roll their own
solution and (hopefully) share it with the community at large.

Remember--we (Dojo) have to support the community at large, and because of
that we have to be a lot more careful about what gets into the actual
library and what doesn't.  But (as I hinted at in the last email), good
things are coming soon :)

trt

-- 
View this message in context: http://www.nabble.com/%28Unofficial%29-FilteringTable-enhancements-tf3023551.html#a8530998
Sent from the Dojo mailing list archive at Nabble.com.