You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Cassie <do...@apache.org> on 2008/04/16 20:05:56 UTC

Re: [jira] Commented: (SHINDIG-200) Patch to construct server from jars and war overlays

Yes, I know, you have mentioned this before, and I completely agree.

-however-

i believe that refactorings shouldn't be done as humongous changes. we
should break them up into small manageable steps in order to make code
reviews more readable and easier to understand. on that note, i think moving
everything into common to get rid of the bad dependency and then
-immediately- renaming the classes and refactoring gadget exception to be
broken up is a good way to go.

on that note though, if you still feel really strongly it would be great if
you can help refactor this and we can do the moving second. i am not as
familiar with the java/gadgets code so its harder for me to undertake the
refactoring step. if you can do the rename and exception split up then i
will update my patch. up to you.

thanks.
- cassie



On Wed, Apr 16, 2008 at 7:47 PM, Kevin Brown (JIRA) <ji...@apache.org> wrote:

>
>    [
> https://issues.apache.org/jira/browse/SHINDIG-200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12589673#action_12589673]
>
> Kevin Brown commented on SHINDIG-200:
> -------------------------------------
>
> GadgetException should definitely *not* be moved to common. ContentFetchers
> should not be throwing GadgetException, they should be throwing a
> ContentFetcherException or something of the sort.
>
> When GadgetToken* is moved, we should rename it to SecurityToken*.
>
> Nothing "Gadget*" should be in the common directory.
>
> > Patch to construct server from jars and war overlays
> > ----------------------------------------------------
> >
> >                 Key: SHINDIG-200
> >                 URL: https://issues.apache.org/jira/browse/SHINDIG-200
> >             Project: Shindig
> >          Issue Type: Improvement
> >            Reporter: Ian Boston
> >         Attachments: create_java_common_without_file_moves.patch,
> serverbuild2.patch
> >
> >
> > From the list:
> > +1 -- I like this a lot, even if it is slightly more complex.
> > On Tue, Apr 15, 2008 at 5:13 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> > Looking at the patch and just having updated from svn, I think this means
> > that
> > in addition to the patch Cassie is working on....
> > java/social-api   becomes a jar
> > java/gadget        becomes a jar
> > ---------
> > There is a new project to build a overlay war from features/**
> > javascript/** config/** ,
> > perhapse split into 2, one for gadgets and one for social-api (not
> certain
> > about the split)
> > eg
> > java/gadget-resources
> > java/social-api-resources
> > These just contain a pom.xml that builds the overlay.
> > ----------------
> > Then there is a new webapp
> > java/server
> > That contains src/main/webapp/WEB-INF/web.xml
> > that unpacks the overlay wars, pulls in the jars and packages as a war...
> > for running in jetty.
> > This would not enforce boundaries between social-api and gadget, but
> would
> > enable both to run, and for others to construct a target.
> > Perhaps thats too complex and there could be some simplification.... I
> > would be happy to generate a patch once Cassie is done....
> > One other thing. I have found that the maven plugins that build sites
> > (less important) and perform releases(more important) are picky about the
> > relationship between the directory name, the artifact ID, how the group
> ID
> > changes as the directory structure gets  deeper.
> > If you want to use these plugins in multiproject mode, then it probably
> > worth trying them now... just to make certain that they do work. If the
> > entire build is maven2 based, the the release plugin is well worth the
> > hassle Changing artifact names and structure later an be a complete pain,
> > see https://source.sakaiproject.org/svn//kernel/trunk/ to see what I
> mean.
> > Ian
> > On 16 Apr 2008, at 00:40, Kevin Brown wrote:
> > On Tue, Apr 15, 2008 at 3:14 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> >  One way might be...
> > add a webapp project that takes a jar from gadgets and a jar from
> > social
> > api and a jar from common and builds the war.
> > That's what I'm in favor of doing.
> >  It might also overlay wars or zips containing all the javascript html
> > and
> > css files.. so that in the war project there is just the web.xml
> > thatis used
> > for a sample deployment.
> > These would belong in there as well, as resources.
> > Then with the jetty target, run that war.
> > For those who want to construct their own webapp and perhaps customize
> > the
> > injection of the service layer, they can pull the gadgets jar, the
> > social-api jar and the javascript zip/jar/war from the maven repo and
> > merge
> > with their own webapp project (with a customized web.xml, service
> > implementation jar and guice service module)
> > And that's exactly what I've been asking for. It's exactly what I want
> > to do
> > for our deployments at google, and I'm sure it's a model that other
> > people
> > would like as well.
> > Ian
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>

Re: [jira] Commented: (SHINDIG-200) Patch to construct server from jars and war overlays

Posted by Kevin Brown <et...@google.com>.
On Wed, Apr 16, 2008 at 11:05 AM, Cassie <do...@apache.org> wrote:

> Yes, I know, you have mentioned this before, and I completely agree.
>
> -however-
>
> i believe that refactorings shouldn't be done as humongous changes. we
> should break them up into small manageable steps in order to make code
> reviews more readable and easier to understand. on that note, i think
> moving
> everything into common to get rid of the bad dependency and then
> -immediately- renaming the classes and refactoring gadget exception to be
> broken up is a good way to go.
>
> on that note though, if you still feel really strongly it would be great
> if
> you can help refactor this and we can do the moving second. i am not as
> familiar with the java/gadgets code so its harder for me to undertake the
> refactoring step. if you can do the rename and exception split up then i
> will update my patch. up to you.


I'd suggest not moving GadgetException -- create a new Exception type (or
types). Moving it would require modifying almost every file in java/gadgets,
which makes little sense when the only reason it would be moved would be for
security tokens and content fetchers.


>
> thanks.
> - cassie
>
>
>
> On Wed, Apr 16, 2008 at 7:47 PM, Kevin Brown (JIRA) <ji...@apache.org>
> wrote:
>
> >
> >    [
> >
> https://issues.apache.org/jira/browse/SHINDIG-200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12589673#action_12589673
> ]
> >
> > Kevin Brown commented on SHINDIG-200:
> > -------------------------------------
> >
> > GadgetException should definitely *not* be moved to common.
> ContentFetchers
> > should not be throwing GadgetException, they should be throwing a
> > ContentFetcherException or something of the sort.
> >
> > When GadgetToken* is moved, we should rename it to SecurityToken*.
> >
> > Nothing "Gadget*" should be in the common directory.
> >
> > > Patch to construct server from jars and war overlays
> > > ----------------------------------------------------
> > >
> > >                 Key: SHINDIG-200
> > >                 URL: https://issues.apache.org/jira/browse/SHINDIG-200
> > >             Project: Shindig
> > >          Issue Type: Improvement
> > >            Reporter: Ian Boston
> > >         Attachments: create_java_common_without_file_moves.patch,
> > serverbuild2.patch
> > >
> > >
> > > From the list:
> > > +1 -- I like this a lot, even if it is slightly more complex.
> > > On Tue, Apr 15, 2008 at 5:13 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> > > Looking at the patch and just having updated from svn, I think this
> means
> > > that
> > > in addition to the patch Cassie is working on....
> > > java/social-api   becomes a jar
> > > java/gadget        becomes a jar
> > > ---------
> > > There is a new project to build a overlay war from features/**
> > > javascript/** config/** ,
> > > perhapse split into 2, one for gadgets and one for social-api (not
> > certain
> > > about the split)
> > > eg
> > > java/gadget-resources
> > > java/social-api-resources
> > > These just contain a pom.xml that builds the overlay.
> > > ----------------
> > > Then there is a new webapp
> > > java/server
> > > That contains src/main/webapp/WEB-INF/web.xml
> > > that unpacks the overlay wars, pulls in the jars and packages as a
> war...
> > > for running in jetty.
> > > This would not enforce boundaries between social-api and gadget, but
> > would
> > > enable both to run, and for others to construct a target.
> > > Perhaps thats too complex and there could be some simplification.... I
> > > would be happy to generate a patch once Cassie is done....
> > > One other thing. I have found that the maven plugins that build sites
> > > (less important) and perform releases(more important) are picky about
> the
> > > relationship between the directory name, the artifact ID, how the
> group
> > ID
> > > changes as the directory structure gets  deeper.
> > > If you want to use these plugins in multiproject mode, then it
> probably
> > > worth trying them now... just to make certain that they do work. If
> the
> > > entire build is maven2 based, the the release plugin is well worth the
> > > hassle Changing artifact names and structure later an be a complete
> pain,
> > > see https://source.sakaiproject.org/svn//kernel/trunk/ to see what I
> > mean.
> > > Ian
> > > On 16 Apr 2008, at 00:40, Kevin Brown wrote:
> > > On Tue, Apr 15, 2008 at 3:14 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> > >  One way might be...
> > > add a webapp project that takes a jar from gadgets and a jar from
> > > social
> > > api and a jar from common and builds the war.
> > > That's what I'm in favor of doing.
> > >  It might also overlay wars or zips containing all the javascript html
> > > and
> > > css files.. so that in the war project there is just the web.xml
> > > thatis used
> > > for a sample deployment.
> > > These would belong in there as well, as resources.
> > > Then with the jetty target, run that war.
> > > For those who want to construct their own webapp and perhaps customize
> > > the
> > > injection of the service layer, they can pull the gadgets jar, the
> > > social-api jar and the javascript zip/jar/war from the maven repo and
> > > merge
> > > with their own webapp project (with a customized web.xml, service
> > > implementation jar and guice service module)
> > > And that's exactly what I've been asking for. It's exactly what I want
> > > to do
> > > for our deployments at google, and I'm sure it's a model that other
> > > people
> > > would like as well.
> > > Ian
> >
> > --
> > This message is automatically generated by JIRA.
> > -
> > You can reply to this email to add a comment to the issue online.
> >
> >
>



-- 
~Kevin