geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jared Stewart <jstew...@pivotal.io>
Subject Re: Review Request 58682: GEODE-2662: Missing keys cause columns to shift in gfsh table display.
Date Mon, 01 May 2017 20:48:40 GMT

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


Ship it!




Ship It!

- Jared Stewart


On April 27, 2017, 6:30 p.m., Patrick Rhomberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58682/
> -----------------------------------------------------------
> 
> (Updated April 27, 2017, 6:30 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and Swapnil
Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Added unittests to capture failing behavior.
> 
> Corrected buildTable shifting columns when adding values with additional keys.
> 
> 
> Refactoring of DataCommandResult and DataCommandFunction
> 
> ------
> 
> The majority of changes to DataCommandFunction and DataCommandResult are refactoring.
 Two ignored exceptions have been explicitly noted in the logger.
> 
> - In DataCommandFunction:423, there's an empty if block.  I imagine I should remove that,
since empty code is a waste.  Looking for it, I couldn't find the comment-hinted separate
method, though. Anyone familiar with this corner of the code know if that actuall happens?
> 
> - Qualitative changes are in DataCommandResult.buildTable.  Items in the table are scanned
to build an agggregate key set, and those items missing any of these keys are padded with
`MISSING_VALUE`.
> 
> - I moved some of the more cumbersome blocks of code to subfunctions, but may have done
this more than necessary.  Opinions?
> 
> - Introduced DataCommandFunctionWithPDXJUnitTest to explicitly drive development / note
the bug in GEODE-2662.  Are there additional tests that would fit naturally in this file?
> 
> 
> Diffs
> -----
> 
>   geode-core/build.gradle f07444a 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java
6324b5c 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
423d781 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
bb77466 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TabularResultData.java
e72654e 
>   geode-core/src/test/java/org/apache/geode/cache/query/dunit/QueryDataInconsistencyDUnitTest.java
1af6261 
>   geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/DataCommandFunctionWithPDXJUnitTest.java
PRE-CREATION 
>   geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java ead1047

> 
> 
> Diff: https://reviews.apache.org/r/58682/diff/5/
> 
> 
> Testing
> -------
> 
> Previous precheckin returned fully green.  Running with new refactoring but expect no
issues.  (Common last words.)
> 
> 
> Thanks,
> 
> Patrick Rhomberg
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message