Re: Review Request 56734: GEODE-2474: refactor code to use SystemUtils to read OS system props

2017-02-15 Thread Kirk Lund


> On Feb. 16, 2017, 12:28 a.m., Kevin Duling wrote:
> > Do you mean to be checking in with TODO comments?

Yes. These are further changes I think need to be made to NetstatFunction to 
harden it. We also need to add streaming to fix GEODE-2488 (OOME) but I didn't 
see an obvious place to add a TODO for that.


- Kirk


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


On Feb. 16, 2017, 12:23 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56734/
> ---
> 
> (Updated Feb. 16, 2017, 12:23 a.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, John Blum, Kevin 
> Duling, and Ken Howe.
> 
> 
> Bugs: GEODE-2474
> https://issues.apache.org/jira/browse/GEODE-2474
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2474: refactor code to use SystemUtils to read OS system props
> 
> Centralize OS system property reading in SystemUtils.
> 
> Refactor NetstatFunction and GemFireVersion to use SystemUtils.
> 
> Fix use of netstat and lsof on Mac OS.
> 
> Add TODOs for future cleanup of NetstatFunction.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/GemFireVersion.java 
> 26d4fb3c5705bffdcdbbc6c261dbe9ffd297642e 
>   geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 
> 66c158c93fecac4feb2da56f617f5efc7bba56e1 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/NetstatFunction.java
>  c435e04dc50ccae50c1008d1153797bbc6ff30f4 
>   
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java
>  48f176eabc18d3ffa56daaa7da12634a9554f39d 
> 
> Diff: https://reviews.apache.org/r/56734/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 56734: GEODE-2474: refactor code to use SystemUtils to read OS system props

2017-02-15 Thread Jared Stewart

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


Ship it!




Ship It!

- Jared Stewart


On Feb. 16, 2017, 12:23 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56734/
> ---
> 
> (Updated Feb. 16, 2017, 12:23 a.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, John Blum, Kevin 
> Duling, and Ken Howe.
> 
> 
> Bugs: GEODE-2474
> https://issues.apache.org/jira/browse/GEODE-2474
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2474: refactor code to use SystemUtils to read OS system props
> 
> Centralize OS system property reading in SystemUtils.
> 
> Refactor NetstatFunction and GemFireVersion to use SystemUtils.
> 
> Fix use of netstat and lsof on Mac OS.
> 
> Add TODOs for future cleanup of NetstatFunction.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/GemFireVersion.java 
> 26d4fb3c5705bffdcdbbc6c261dbe9ffd297642e 
>   geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 
> 66c158c93fecac4feb2da56f617f5efc7bba56e1 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/NetstatFunction.java
>  c435e04dc50ccae50c1008d1153797bbc6ff30f4 
>   
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java
>  48f176eabc18d3ffa56daaa7da12634a9554f39d 
> 
> Diff: https://reviews.apache.org/r/56734/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Re: Review Request 56734: GEODE-2474: refactor code to use SystemUtils to read OS system props

2017-02-15 Thread Kevin Duling

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


Ship it!




Do you mean to be checking in with TODO comments?

- Kevin Duling


On Feb. 15, 2017, 4:23 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56734/
> ---
> 
> (Updated Feb. 15, 2017, 4:23 p.m.)
> 
> 
> Review request for geode, Jinmei Liao, Jared Stewart, John Blum, Kevin 
> Duling, and Ken Howe.
> 
> 
> Bugs: GEODE-2474
> https://issues.apache.org/jira/browse/GEODE-2474
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2474: refactor code to use SystemUtils to read OS system props
> 
> Centralize OS system property reading in SystemUtils.
> 
> Refactor NetstatFunction and GemFireVersion to use SystemUtils.
> 
> Fix use of netstat and lsof on Mac OS.
> 
> Add TODOs for future cleanup of NetstatFunction.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/GemFireVersion.java 
> 26d4fb3c5705bffdcdbbc6c261dbe9ffd297642e 
>   geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 
> 66c158c93fecac4feb2da56f617f5efc7bba56e1 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/NetstatFunction.java
>  c435e04dc50ccae50c1008d1153797bbc6ff30f4 
>   
> geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java
>  48f176eabc18d3ffa56daaa7da12634a9554f39d 
> 
> Diff: https://reviews.apache.org/r/56734/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>



Review Request 56734: GEODE-2474: refactor code to use SystemUtils to read OS system props

2017-02-15 Thread Kirk Lund

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

Review request for geode, Jinmei Liao, Jared Stewart, John Blum, Kevin Duling, 
and Ken Howe.


Bugs: GEODE-2474
https://issues.apache.org/jira/browse/GEODE-2474


Repository: geode


Description
---

GEODE-2474: refactor code to use SystemUtils to read OS system props

Centralize OS system property reading in SystemUtils.

Refactor NetstatFunction and GemFireVersion to use SystemUtils.

Fix use of netstat and lsof on Mac OS.

Add TODOs for future cleanup of NetstatFunction.


Diffs
-

  geode-core/src/main/java/org/apache/geode/internal/GemFireVersion.java 
26d4fb3c5705bffdcdbbc6c261dbe9ffd297642e 
  geode-core/src/main/java/org/apache/geode/internal/lang/SystemUtils.java 
66c158c93fecac4feb2da56f617f5efc7bba56e1 
  
geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/NetstatFunction.java
 c435e04dc50ccae50c1008d1153797bbc6ff30f4 
  
geode-core/src/test/java/org/apache/geode/internal/lang/SystemUtilsJUnitTest.java
 48f176eabc18d3ffa56daaa7da12634a9554f39d 

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


Testing
---

precheckin passed


Thanks,

Kirk Lund