I see what your saying now.  I have thought about this a little, but I 
think it is a larger problem.  It would be nice to set the context root in 
one location, like shindig.properties, and everything else pull from 
there.  I don't know why we have to change several properties to 
accomplish this.  It's something I may look into more, but at the moment 
it's not blocking me so I am moving on to other problems.  The fix, which 
Paul has already delivered to the trunk, just makes it a little easier in 
the container to change the context root.  In general I agree though, we 
shouldn't have to make so many changes.

-Ryan

Email: [email protected]
Phone: 978-899-3041
developerWorks Profile



From:   Maxwell <[email protected]>
To:     "[email protected]" <[email protected]>
Date:   12/21/2010 09:13 PM
Subject:        Re: Fix For SHINDIG-1484 (issue3811041)



When we need to change context, we have to change properties in 
shindig.properties and container,já file (or create custom files, or 
inject using another methods than files,

And we still have to override the constructor in server base, with your 
change we do not need to override anymore, but need to set the property, 
so maybe you can get this property from one of these files, or another 
injected value.

Understood? I do not have the code now to explain better.

Sent from my iPhone

On 21/12/2010, at 23:51, "Ryan J Baxter" <[email protected]> wrote:

> Hi Maxwell,
> 
> Not sure I understand what you are saying.  What property in 
container.js 
> are you referring to?
> 
> -Ryan
> 
> Email: [email protected]
> Phone: 978-899-3041
> developerWorks Profile
> 
> 
> 
> From:   Maxwell <[email protected]>
> To:     [email protected]
> Date:   12/21/2010 08:20 PM
> Subject:        Re: Fw: Fix For SHINDIG-1484 (issue3811041)
> 
> 
> 
> Hey Ryan,
> 
> In the case you set the serverBase, you could get the value from
> container.js, will help people that need to change context to a 
non-root.
> 
> Maxwell
> 
> On Tue, Dec 21, 2010 at 11:02 PM, Ryan J Baxter <[email protected]> 
> wrote:
> 
>> Hi All,
>> 
>> Could someone please code review this fix for me.  Its a very small
>> change, so shouldn't take to much effort.  Thanks.
>> 
>> -Ryan
>> 
>> Email: [email protected]
>> Phone: 978-899-3041
>> developerWorks Profile
>> ----- Forwarded by Ryan J Baxter/Westford/IBM on 12/21/2010 08:01 PM 
> -----
>> 
>> From:   [email protected]
>> To:     [email protected]
>> Cc:     Ryan J Baxter/Westford/i...@lotus, 
> [email protected]
>> Date:   12/20/2010 09:29 PM
>> Subject:        Fix For SHINDIG-1484 (issue3811041)
>> 
>> 
>> 
>> Reviewers: dev-remailer_shindig.apache.org,
>> 
>> Description:
>> When you extend shindig.BaseIfrGadget in your own container the above
>> code makes it difficult to override serverBase_.
>> https://issues.apache.org/jira/browse/SHINDIG-1484
>> 
>> Please review this at http://codereview.appspot.com/3811041/
>> 
>> Affected files:
>> 
>> 
>> 
> 
features/src/main/javascript/features/shindig.container/shindig-container.js
>> 
>> 
>> ### Eclipse Workspace Patch 1.0
>> #P shindig-project
>> Index:
>> 
>> 
> 
features/src/main/javascript/features/shindig.container/shindig-container.js
>> ===================================================================
>> ---
>> 
>> 
> 
features/src/main/javascript/features/shindig.container/shindig-container.js
>> 
>> (revision 1050262)
>> +++
>> 
>> 
> 
features/src/main/javascript/features/shindig.container/shindig-container.js
>> 
>> (working copy)
>> @@ -479,7 +479,9 @@
>> 
>> shindig.BaseIfrGadget = function(opt_params) {
>>   shindig.Gadget.call(this, opt_params);
>> -  this.serverBase_ = '/gadgets/'; // default gadget server
>> +  if(!this.serverBase_){
>> +                  this.serverBase_ = '/gadgets/'; // default gadget
>> server
>> +  }
>>   this.queryIfrGadgetType_();
>> };
>> 
>> 
>> 
>> 
>> 
>> 
> 
> 
> 



Reply via email to