You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2009/09/22 23:59:58 UTC
Extensible ModulePrefs, better visitor pattern
One nit, but the majority concern I have is in the overall result of
this approach. Seems to me this encourages ad hoc modification of the
spec, leading to inconsistencies and incompatibility between gadgets.
Why not use the existing <Requires>/<Optional> syntax? It's limited, but
typically can get the job done, using a known pattern.
http://codereview.appspot.com/121064/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
(right):
http://codereview.appspot.com/121064/diff/1/2#newcode26
Line 26: import org.w3c.dom.*;
please expand these wildcards, verbose as it may be.
http://codereview.appspot.com/121064
Re: Extensible ModulePrefs, better visitor pattern
Posted by John Hjelmstad <fa...@google.com>.
I get the rationale behind the implementation details here -- what I don't
have insight into are what extensions are needed that can't be achieved
within the existing spec/extension mechanism.
Thx for cleaning up the imports.
On Tue, Sep 22, 2009 at 3:07 PM, Paul Lindner <li...@inuus.com> wrote:
> On Tue, Sep 22, 2009 at 2:59 PM, <jo...@gmail.com> wrote:
>
> > One nit, but the majority concern I have is in the overall result of
> > this approach. Seems to me this encourages ad hoc modification of the
> > spec, leading to inconsistencies and incompatibility between gadgets.
> > Why not use the existing <Requires>/<Optional> syntax? It's limited, but
> > typically can get the job done, using a known pattern.
> >
>
> I agree in principal, however in practice we've seen the need at the
> implementer level to add extensions in this area. Henning has a patch that
> implements a stax-based parser that allowed for namespaced extensions. I
> didn't have time to test and merge that patch so I went ahead and added in
> the functionality you see.
>
>
>
> > http://codereview.appspot.com/121064/diff/1/2
> > File
> >
> java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
> > (right):
> >
> > http://codereview.appspot.com/121064/diff/1/2#newcode26
> > Line 26: import org.w3c.dom.*;
> > please expand these wildcards, verbose as it may be.
> >
> >
> Silly IDE.. I'll fix it..
>
>
> > http://codereview.appspot.com/121064
> >
>
Re: Extensible ModulePrefs, better visitor pattern
Posted by Paul Lindner <li...@inuus.com>.
On Tue, Sep 22, 2009 at 2:59 PM, <jo...@gmail.com> wrote:
> One nit, but the majority concern I have is in the overall result of
> this approach. Seems to me this encourages ad hoc modification of the
> spec, leading to inconsistencies and incompatibility between gadgets.
> Why not use the existing <Requires>/<Optional> syntax? It's limited, but
> typically can get the job done, using a known pattern.
>
I agree in principal, however in practice we've seen the need at the
implementer level to add extensions in this area. Henning has a patch that
implements a stax-based parser that allowed for namespaced extensions. I
didn't have time to test and merge that patch so I went ahead and added in
the functionality you see.
> http://codereview.appspot.com/121064/diff/1/2
> File
> java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/ModulePrefs.java
> (right):
>
> http://codereview.appspot.com/121064/diff/1/2#newcode26
> Line 26: import org.w3c.dom.*;
> please expand these wildcards, verbose as it may be.
>
>
Silly IDE.. I'll fix it..
> http://codereview.appspot.com/121064
>