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

(Updated April 24, 2017, 8:06 p.m.)


Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, and 
Swapnil Bawaskar.


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

GEODE-2662: Missing keys cause columns to shift in gfsh table display.


Repository: geode


Description (updated)
-------

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 f07444af7baa41fc0fcd0a088521bd96b317d15f 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/domain/DataCommandResult.java
 423d781248458a450c2e98cd8a0e609c8656edb0 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DataCommandFunction.java
 bb77466962a9a07a48bc09976b94d2f3822aed6d 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/result/TabularResultData.java
 e72654ed4aa11a5eb3f69a275d253cbedeb7df1e 
  
geode-core/src/test/java/org/apache/geode/cache/query/dunit/QueryDataInconsistencyDUnitTest.java
 1af6261613f51a7cc3afe49bf8f3e2785ed2d60d 
  
geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/DataCommandFunctionWithPDXJUnitTest.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/58682/diff/1/


Testing
-------

precheckin currently running.


Thanks,

Patrick Rhomberg

Reply via email to