You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2016/03/16 02:23:08 UTC

Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/
-----------------------------------------------------------

Review request for geode, Anthony Baker and Jason Huynh.


Repository: geode


Description
-------

This allows us to make sure we don't accidentally start shipping jars
that we didn't intend to.


Diffs
-----

  geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
  geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 

Diff: https://reviews.apache.org/r/44877/diff/


Testing
-------


Thanks,

Dan Smith


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review124077
-----------------------------------------------------------


Ship it!




Ship It!

- Mark Bretl


On March 16, 2016, 2:34 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 2:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review124047
-----------------------------------------------------------


Ship it!




Ship It!

- Jason Huynh


On March 16, 2016, 9:34 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 9:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Dan Smith <ds...@pivotal.io>.

> On March 18, 2016, 2:27 p.m., Jens Deppe wrote:
> > I know this is a bit OC, but please could you sort the expected_jars.txt file :)

No problem. I'll fix that.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review124188
-----------------------------------------------------------


On March 16, 2016, 9:34 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 9:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Jens Deppe <jd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review124188
-----------------------------------------------------------


Ship it!




I know this is a bit OC, but please could you sort the expected_jars.txt file :)

- Jens Deppe


On March 16, 2016, 9:34 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 9:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/
-----------------------------------------------------------

(Updated March 16, 2016, 9:34 p.m.)


Review request for geode, Anthony Baker and Jason Huynh.


Changes
-------

Excluding versions from the expected jars list


Repository: geode


Description
-------

This allows us to make sure we don't accidentally start shipping jars
that we didn't intend to.


Diffs (updated)
-----

  geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
  geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 

Diff: https://reviews.apache.org/r/44877/diff/


Testing
-------


Thanks,

Dan Smith


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Dan Smith <ds...@pivotal.io>.

> On March 16, 2016, 8:17 p.m., Mark Bretl wrote:
> > Can anything be done not to have static versions of jars in the expected_jars.txt? This now introduces having to update two different places again for dependencies and if we move back to transitive dependencies, then we have to manage those as well. I am definitely for making sure we keep tracking of adding dependencies and what we include in the distribution, however, I can see this getting out of sync. Myabe that is way it needs to be though.

Good point. I was a little worried that a dependency could change licenses as it changes versions. But the chance that someone would actually notice that even with this test is slim, so it's probably just better to not check the versions.

I do want to have this test, because now we could be shipping dependencies that we don't declare in our .gradle files because they are transitive dependencies.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review123921
-----------------------------------------------------------


On March 16, 2016, 9:34 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 9:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Mark Bretl <mb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review123921
-----------------------------------------------------------



Can anything be done not to have static versions of jars in the expected_jars.txt? This now introduces having to update two different places again for dependencies and if we move back to transitive dependencies, then we have to manage those as well. I am definitely for making sure we keep tracking of adding dependencies and what we include in the distribution, however, I can see this getting out of sync. Myabe that is way it needs to be though.

- Mark Bretl


On March 15, 2016, 6:23 p.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 15, 2016, 6:23 p.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by Dan Smith <ds...@pivotal.io>.

> On March 16, 2016, 6:28 p.m., anilkumar gingade wrote:
> > geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java, line 78
> > <https://reviews.apache.org/r/44877/diff/1/?file=1300497#file1300497line78>
> >
> >     Does this work on Windows....Or can we ignore windows platform?

I think it should - that's just a regular expression for files ending in .jar.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review123905
-----------------------------------------------------------


On March 16, 2016, 1:23 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 1:23 a.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>


Re: Review Request 44877: GEODE-27: Adding a test to verify the bundled jars have not changed

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44877/#review123905
-----------------------------------------------------------


Fix it, then Ship it!




Ship It!


geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java (line 78)
<https://reviews.apache.org/r/44877/#comment186198>

    Does this work on Windows....Or can we ignore windows platform?


- anilkumar gingade


On March 16, 2016, 1:23 a.m., Dan Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44877/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 1:23 a.m.)
> 
> 
> Review request for geode, Anthony Baker and Jason Huynh.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> This allows us to make sure we don't accidentally start shipping jars
> that we didn't intend to.
> 
> 
> Diffs
> -----
> 
>   geode-assembly/src/test/java/com/gemstone/gemfire/BundledJarsJUnitTest.java PRE-CREATION 
>   geode-assembly/src/test/resources/expected_jars.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44877/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Smith
> 
>