You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jens Deppe <jd...@pivotal.io> on 2016/05/09 17:31:30 UTC
Review Request 47129: GEODE-1361: Provide location of geode-web war
for use in geode-core
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/
-----------------------------------------------------------
Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
Repository: geode
Description
-------
GEODE-1361: Provide location of geode-web war for use in geode-core
Diffs
-----
geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
Diff: https://reviews.apache.org/r/47129/diff/
Testing
-------
Thanks,
Jens Deppe
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Jens Deppe <jd...@pivotal.io>.
> On May 9, 2016, 5:43 p.m., Anthony Baker wrote:
> > geode-core/build.gradle, line 130
> > <https://reviews.apache.org/r/47129/diff/1/?file=1376862#file1376862line130>
> >
> > Does this create a circular dependency between geode-core and geode-web?
No, I think it's OK. geode-web_main depends on geode-core_main but it's geode-core_tests that depend on geode-web_main.
- Jens
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132301
-----------------------------------------------------------
On May 9, 2016, 5:31 p.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 5:31 p.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Anthony Baker <ab...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132301
-----------------------------------------------------------
geode-core/build.gradle (line 130)
<https://reviews.apache.org/r/47129/#comment196480>
Does this create a circular dependency between geode-core and geode-web?
- Anthony Baker
On May 9, 2016, 5:31 p.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 5:31 p.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Jens Deppe <jd...@pivotal.io>.
> On May 9, 2016, 7:03 p.m., Mark Bretl wrote:
> > geode-core/build.gradle, line 130
> > <https://reviews.apache.org/r/47129/diff/1/?file=1376862#file1376862line130>
> >
> > Same thoughts as Anthony, this looks to add a circular dependency.
> >
> > If this is the case, feels like tests are in the wrong project...
I could see why you think that, however the reason for this is that we're try to enable http-based gfsh testing for exactly the same test code which uses regular gfsh access (jmx). We're doiong this by parameterizing the JUnit tests. We could split them up, but then we'd end up with identical code duplicated in -core and -assembly. I also don't think it's quite appropriate to move all the tests into -assembly as they're not relying on a full product tree.
- Jens
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132309
-----------------------------------------------------------
On May 9, 2016, 5:31 p.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 5:31 p.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Jens Deppe <jd...@pivotal.io>.
> On May 9, 2016, 7:03 p.m., Mark Bretl wrote:
> > geode-core/build.gradle, line 130
> > <https://reviews.apache.org/r/47129/diff/1/?file=1376862#file1376862line130>
> >
> > Same thoughts as Anthony, this looks to add a circular dependency.
> >
> > If this is the case, feels like tests are in the wrong project...
>
> Jens Deppe wrote:
> I could see why you think that, however the reason for this is that we're try to enable http-based gfsh testing for exactly the same test code which uses regular gfsh access (jmx). We're doiong this by parameterizing the JUnit tests. We could split them up, but then we'd end up with identical code duplicated in -core and -assembly. I also don't think it's quite appropriate to move all the tests into -assembly as they're not relying on a full product tree.
>
> Dan Smith wrote:
> Would it be cleaner just add tests to geode-web that extend the geode-core tests? That way you wouldn't duplicate the code, but you wouldn't introduce a circular dependencies of geode-web->geode-core->geode-web.
OK, I think we'll do that.
- Jens
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132309
-----------------------------------------------------------
On May 9, 2016, 5:31 p.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 5:31 p.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Dan Smith <ds...@pivotal.io>.
> On May 9, 2016, 7:03 p.m., Mark Bretl wrote:
> > geode-core/build.gradle, line 130
> > <https://reviews.apache.org/r/47129/diff/1/?file=1376862#file1376862line130>
> >
> > Same thoughts as Anthony, this looks to add a circular dependency.
> >
> > If this is the case, feels like tests are in the wrong project...
>
> Jens Deppe wrote:
> I could see why you think that, however the reason for this is that we're try to enable http-based gfsh testing for exactly the same test code which uses regular gfsh access (jmx). We're doiong this by parameterizing the JUnit tests. We could split them up, but then we'd end up with identical code duplicated in -core and -assembly. I also don't think it's quite appropriate to move all the tests into -assembly as they're not relying on a full product tree.
Would it be cleaner just add tests to geode-web that extend the geode-core tests? That way you wouldn't duplicate the code, but you wouldn't introduce a circular dependencies of geode-web->geode-core->geode-web.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132309
-----------------------------------------------------------
On May 9, 2016, 5:31 p.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 5:31 p.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>
Re: Review Request 47129: GEODE-1361: Provide location of geode-web
war for use in geode-core
Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47129/#review132309
-----------------------------------------------------------
geode-core/build.gradle (line 130)
<https://reviews.apache.org/r/47129/#comment196485>
Same thoughts as Anthony, this looks to add a circular dependency.
If this is the case, feels like tests are in the wrong project...
- Mark Bretl
On May 9, 2016, 10:31 a.m., Jens Deppe wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47129/
> -----------------------------------------------------------
>
> (Updated May 9, 2016, 10:31 a.m.)
>
>
> Review request for geode, Jinmei Liao, Kirk Lund, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1361: Provide location of geode-web war for use in geode-core
>
>
> Diffs
> -----
>
> geode-core/build.gradle 45051dc771452bf06ba6708efd8e1361791e13e8
> geode-web/build.gradle 5bd1d8949c1b1379ddd6a1bd8ab2751ce73a79fd
>
> Diff: https://reviews.apache.org/r/47129/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jens Deppe
>
>