You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/02/06 01:22:14 UTC
Review Request 43275: GEODE-913: refactor AbstractDistributionConfig
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/
-----------------------------------------------------------
Review request for geode.
Repository: geode
Description
-------
* use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
* delete multiple huge if/else statements to use annoations
* delete unnecessary checks and ifModifiables since they all use default implementations.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeIsModifiable.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/43275/diff/
Testing
-------
Thanks,
Jinmei Liao
Re: Review Request 43275: GEODE-913: refactor
AbstractDistributionConfig
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/#review118115
-----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java (line 1)
<https://reviews.apache.org/r/43275/#comment179349>
On all these new files you need to copyright header
- Darrel Schneider
On Feb. 5, 2016, 4:49 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43275/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2016, 4:49 p.m.)
>
>
> Review request for geode.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
> * delete multiple huge if/else statements to use annoations
> * delete unnecessary checks and ifModifiables since they all use default implementations.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeIsModifiable.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43275/diff/
>
>
> Testing
> -------
>
> unit tests
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 43275: GEODE-913: refactor
AbstractDistributionConfig
Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/#review118937
-----------------------------------------------------------
Fix it, then Ship it!
Fix formatting where needed and then ship it
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java (line 43)
<https://reviews.apache.org/r/43275/#comment180244>
I just realized you're using 4 space indentation here. The style guidelines is supposedly 2 space, soft tabs (I'm not sure how to set this in intellij but in the root of a geode checkout is etc/ which contains intellijIdeaCodeStyle.xml which you should be able to import and use for formatting code.
- Kirk Lund
On Feb. 11, 2016, 3:41 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43275/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 3:41 p.m.)
>
>
> Review request for geode.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
> * delete multiple huge if/else statements to use annoations
> * delete unnecessary checks and ifModifiables since they all use default implementations.
>
>
> Diffs
> -----
>
> gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
> gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
> gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
> gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
> gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/43275/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 43275: GEODE-913: refactor
AbstractDistributionConfig
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/
-----------------------------------------------------------
(Updated Feb. 11, 2016, 3:41 p.m.)
Review request for geode.
Changes
-------
ran all the precheckin tests and minor modifications
Repository: geode
Description
-------
* use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
* delete multiple huge if/else statements to use annoations
* delete unnecessary checks and ifModifiables since they all use default implementations.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/43275/diff/
Testing (updated)
-------
precheckin
Thanks,
Jinmei Liao
Re: Review Request 43275: GEODE-913: refactor
AbstractDistributionConfig
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/
-----------------------------------------------------------
(Updated Feb. 6, 2016, 5:04 a.m.)
Review request for geode.
Changes
-------
add the license headers, add more tests and deleted all the isXXXModifiable methods
Repository: geode
Description
-------
* use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
* delete multiple huge if/else statements to use annoations
* delete unnecessary checks and ifModifiables since they all use default implementations.
Diffs (updated)
-----
gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/43275/diff/
Testing
-------
unit tests
Thanks,
Jinmei Liao
Re: Review Request 43275: GEODE-913: refactor
AbstractDistributionConfig
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43275/
-----------------------------------------------------------
(Updated Feb. 6, 2016, 12:49 a.m.)
Review request for geode.
Repository: geode
Description
-------
* use annotations to annotate the config attributes and its setters/getters/isModiables/checkers
* delete multiple huge if/else statements to use annoations
* delete unnecessary checks and ifModifiables since they all use default implementations.
Diffs
-----
gemfire-core/src/main/java/com/gemstone/gemfire/admin/jmx/internal/AgentConfigImpl.java 9d602f6aab5f53d2576804ce9f7639b675a0d957
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/AbstractDistributionConfig.java 0cc76ec7a9a9838c12692ea962def0c3b5d5867d
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttribute.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeChecker.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeDesc.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeGetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeIsModifiable.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/ConfigAttributeSetter.java PRE-CREATION
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfig.java f8ff21a0ae29bacfabea1941210f2457b1d652e9
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/DistributionConfigImpl.java 190a1377b53493633427af29f849bc0dc71ef772
gemfire-core/src/main/java/com/gemstone/gemfire/distributed/internal/RuntimeDistributionConfigImpl.java 299744a958c6e7c289f339b0107ad0b7329ed865
gemfire-core/src/main/java/com/gemstone/gemfire/internal/AbstractConfig.java ddf29700d1c404521140114ebad7fd94a8511068
gemfire-core/src/main/java/com/gemstone/gemfire/internal/ConfigSource.java 08f51e4f8fe149b1efdbb9ef6dece58c7fe8cfe6
gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogConfig.java 8c8dd5c522aa6ac5446c1f0932cb638aa1d42444
gemfire-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java PRE-CREATION
gemfire-core/src/test/java/com/gemstone/gemfire/internal/ConfigSourceJUnitTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/43275/diff/
Testing (updated)
-------
unit tests
Thanks,
Jinmei Liao