You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jason Huynh <hu...@gmail.com> on 2015/10/06 19:01:05 UTC

Review Request 39055: Additional fix for GEODE-280

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

Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, and Jianxia Chen.


Repository: geode


Description
-------

The previous refactor was a bit aggressive and did not account for struct comparison (I assumed struct equals method would work just fine but apparently not...)  It did not show in this specific test but was reflected in 5 failing tests overnight.


Diffs
-----

  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java d53c28b 

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


Testing
-------

The failing tests QueryREUpdateInProgressJUnitTest and PRColocatedEquiJoinDUnitTest


Thanks,

Jason Huynh


Re: Review Request 39055: Fix GEODE-384

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39055/#review101666
-----------------------------------------------------------



gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java (line 422)
<https://reviews.apache.org/r/39055/#comment159124>

    It looks like this method may throw NullPointerException if o1 or o2 are null.
    At the least add a comment saying so.


- Darrel Schneider


On Oct. 6, 2015, 10:10 a.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39055/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 10:10 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, and Jianxia Chen.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The previous refactor was a bit aggressive and did not account for struct comparison (I assumed struct equals method would work just fine but apparently not...)  It did not show in this specific test but was reflected in 5 failing tests overnight.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java d53c28b 
> 
> Diff: https://reviews.apache.org/r/39055/diff/
> 
> 
> Testing
> -------
> 
> The failing tests QueryREUpdateInProgressJUnitTest and PRColocatedEquiJoinDUnitTest
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>


Re: Review Request 39055: Fix GEODE-384

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39055/#review101669
-----------------------------------------------------------

Ship it!


Ship It!

- Darrel Schneider


On Oct. 6, 2015, 10:19 a.m., Jason Huynh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39055/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 10:19 a.m.)
> 
> 
> Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, and Jianxia Chen.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The previous refactor was a bit aggressive and did not account for struct comparison (I assumed struct equals method would work just fine but apparently not...)  It did not show in this specific test but was reflected in 5 failing tests overnight.
> 
> 
> Diffs
> -----
> 
>   gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java d53c28b 
> 
> Diff: https://reviews.apache.org/r/39055/diff/
> 
> 
> Testing
> -------
> 
> The failing tests QueryREUpdateInProgressJUnitTest and PRColocatedEquiJoinDUnitTest
> 
> 
> Thanks,
> 
> Jason Huynh
> 
>


Re: Review Request 39055: Fix GEODE-384

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

(Updated Oct. 6, 2015, 5:19 p.m.)


Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, and Jianxia Chen.


Changes
-------

Added comments as suggested
Calling objectsEqual in multiple locations


Repository: geode


Description
-------

The previous refactor was a bit aggressive and did not account for struct comparison (I assumed struct equals method would work just fine but apparently not...)  It did not show in this specific test but was reflected in 5 failing tests overnight.


Diffs (updated)
-----

  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java d53c28b 

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


Testing
-------

The failing tests QueryREUpdateInProgressJUnitTest and PRColocatedEquiJoinDUnitTest


Thanks,

Jason Huynh


Re: Review Request 39055: Fix GEODE-384

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

(Updated Oct. 6, 2015, 5:10 p.m.)


Review request for geode, anilkumar gingade, Bruce Schuchardt, Darrel Schneider, and Jianxia Chen.


Changes
-------

Created new Jira ticket (GEODE-384)


Summary (updated)
-----------------

Fix GEODE-384


Repository: geode


Description
-------

The previous refactor was a bit aggressive and did not account for struct comparison (I assumed struct equals method would work just fine but apparently not...)  It did not show in this specific test but was reflected in 5 failing tests overnight.


Diffs
-----

  gemfire-core/src/test/java/com/gemstone/gemfire/cache/query/functional/StructSetOrResultsSet.java d53c28b 

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


Testing
-------

The failing tests QueryREUpdateInProgressJUnitTest and PRColocatedEquiJoinDUnitTest


Thanks,

Jason Huynh