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




geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java
Lines 265 (patched)
<https://reviews.apache.org/r/58080/#comment243492>

    I think you probably intended to reference the constants declared below 
here.



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java
Lines 24 (patched)
<https://reviews.apache.org/r/58080/#comment243488>

    Is there a reason you chose to use DataSerializable instead the Java 
built-in Serializable?  Just curious as I'm not too familiar with 
DataSerializable.



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java
Lines 60 (patched)
<https://reviews.apache.org/r/58080/#comment243489>

    Might want to make this class final if using getClass() in the equals 
implementation.  (Effective Java has a big discussion about this if you're 
interested)



geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java
Lines 77 (patched)
<https://reviews.apache.org/r/58080/#comment243468>

    I think this might be simpler implementation: 
    ```
    public int hashCode(){
        return Objects.hash(logsSize, diskAvailable, diskSize);
    }
    ```


- Jared Stewart


On March 30, 2017, 2:43 p.m., Ken Howe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58080/
> -----------------------------------------------------------
> 
> (Updated March 30, 2017, 2:43 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Patrick Rhomberg.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> - New Function class added with tests
> - Code to invoke new function added, but disabled, in ExportLogsCommand
> - New option added, but disabled, for specifying size limit for exported
>   logs
> - Refactoring: extracted the interceptor from ExportLogsCommand
> - Refactoring: renamed ExportLogCommand to ExportLogsCommand
> - New Serializable class to return the log size estimate to the command
> - Tests for the log sizing related classes
> 
> This is work-in-progress that is adds sizing estimation for exported logs, 
> but the code is currently not called by ExportLogsCommand. Follow-on work 
> will use the new sizing estimation to provide warnings to the user if 
> exporting logs may fill up disks on servers or the locator.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogCommand.java
>  3f147c19a128dce78c51c31e6758e517cd2ab496 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptor.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunction.java
>  cbdf1c4bc28554a8fbec3740c566ee07c69b4ac9 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfo.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
>  5b1f089c18c404f64929398f6015839eb783ccb4 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/util/LogSizer.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
>  95edd426da8b8f39bb1486661d8c307d43f170d6 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java
>  268fa397db253f12c0effdbf6faa5e822730144c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsInterceptorJUnitTest.java
>  573701fd7f48c8c1b72c0f71993a4adea7bdbe8b 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
>  5a4d274a751a6e22fcfbc0c5e6bd5dff39e3b938 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/LogLevelInterceptorTest.java
>  41b971534fd83c87f33751fa46454e158315f6ae 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportedLogsSizeInfoTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionCacheTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionFileTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsTestSuite.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogSizerTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
>  6ae82da58923fdf3a44217c1773a40d8ff1001bc 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
>  2f5a5cdf8c59e90625fe01c3306462e9277ab4e1 
>   
> geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt
>  397ec14c0bbbea277d378eefcaecfdb90cc9e224 
> 
> 
> Diff: https://reviews.apache.org/r/58080/diff/1/
> 
> 
> Testing
> -------
> 
> Precheckin is running
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Reply via email to