You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Dan Dumont <dd...@us.ibm.com> on 2011/11/28 21:27:24 UTC
Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/
-----------------------------------------------------------
Review request for shindig, Ryan Baxter and Stanton Sievers.
Summary
-------
I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
Please, if I've misinterpreted or misrepresented a config setting, let me know.
Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
Diff: https://reviews.apache.org/r/2950/diff
Testing
-------
All tests pass.
Thanks,
Dan
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/
-----------------------------------------------------------
(Updated 2011-11-29 15:11:09.163058)
Review request for shindig, Ryan Baxter and Stanton Sievers.
Changes
-------
Committed r1207899
Summary
-------
I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
Please, if I've misinterpreted or misrepresented a config setting, let me know.
Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
This addresses bug SHINDIG-1663.
https://issues.apache.org/jira/browse/SHINDIG-1663
Diffs (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
http://svn.apache.org/repos/asf/shindig/trunk/java/common/target/classes/containers/default/container.js PRE-CREATION
http://svn.apache.org/repos/asf/shindig/trunk/php/test/gadgets/ContainerConfigTest.php 1207269
Diff: https://reviews.apache.org/r/2950/diff
Testing
-------
All tests pass.
Thanks,
Dan
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3540
-----------------------------------------------------------
Ship it!
LGTM
- Stanton
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3542
-----------------------------------------------------------
Ship it!
+1
- Henry
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2011-11-29 13:09:24, Jesse Ciancetta wrote:
> > Nevermind my comment about the new "default.domain.*" properties -- after another look this morning with fresh eyes I see that they are used within container.js itself...
> >
> > I did a quick search through the codebase for "jsUriTemplate" and I found a reference in a test PHP file -- if you remove it from container.js it might make sense to remove it from the PHP file too.
> >
> > Assuming you've tested the changes in an actual locked domain environment to be sure everything is still working properly then I say LGTM.
Ok. I'll update the test and then commit. TY for the reviews, everyone. :)
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3554
-----------------------------------------------------------
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Jesse Ciancetta <jc...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3554
-----------------------------------------------------------
Ship it!
Nevermind my comment about the new "default.domain.*" properties -- after another look this morning with fresh eyes I see that they are used within container.js itself...
I did a quick search through the codebase for "jsUriTemplate" and I found a reference in a test PHP file -- if you remove it from container.js it might make sense to remove it from the PHP file too.
Assuming you've tested the changes in an actual locked domain environment to be sure everything is still working properly then I say LGTM.
- Jesse
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3541
-----------------------------------------------------------
Ship it!
LGTM
- Ryan
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Dan Dumont <dd...@us.ibm.com>.
> On 2011-11-28 21:20:14, Jesse Ciancetta wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js, line 86
> > <https://reviews.apache.org/r/2950/diff/1/?file=60293#file60293line86>
> >
> > I dont think I see this anywhere in the updated changes -- is it just not being used anywhere?
I didn't see any references. It looks like it might be used by a client side script loader, but google has not contributed theirs (yet?), so I don't think it's being used.
> On 2011-11-28 21:20:14, Jesse Ciancetta wrote:
> > http://svn.apache.org/repos/asf/shindig/trunk/config/container.js, line 120
> > <https://reviews.apache.org/r/2950/diff/1/?file=60293#file60293line120>
> >
> > All of the "default.domain.*" properties look new to me and I cant find any reference to them in the current codebase -- are those supposed to be added as part of this cleanup?
These aren't used in the source code. They are referenced by other properties below them.
The container lets you use expressions to refer to other variables. Since I've had to change several properties to get locked domains working, I figured referring them all to 1 value might make someone's life easier.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3543
-----------------------------------------------------------
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>
Re: Review Request: Update container config values,
comments. Make things easier to configure for locked domains.
Posted by Jesse Ciancetta <jc...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2950/#review3543
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/config/container.js
<https://reviews.apache.org/r/2950/#comment7879>
I dont think I see this anywhere in the updated changes -- is it just not being used anywhere?
http://svn.apache.org/repos/asf/shindig/trunk/config/container.js
<https://reviews.apache.org/r/2950/#comment7878>
All of the "default.domain.*" properties look new to me and I cant find any reference to them in the current codebase -- are those supposed to be added as part of this cleanup?
- Jesse
On 2011-11-28 20:27:24, Dan Dumont wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2950/
> -----------------------------------------------------------
>
> (Updated 2011-11-28 20:27:24)
>
>
> Review request for shindig, Ryan Baxter and Stanton Sievers.
>
>
> Summary
> -------
>
> I've attempted to make it a little easier for people to enable locked domains by clarifying some comments and marking critical configuration sections.
>
> Please, if I've misinterpreted or misrepresented a config setting, let me know.
>
> Will attach a JIRA if necessary. I did want some comments before submitting something like this though.
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1207269
>
> Diff: https://reviews.apache.org/r/2950/diff
>
>
> Testing
> -------
>
> All tests pass.
>
>
> Thanks,
>
> Dan
>
>