-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33488/
-----------------------------------------------------------
(Updated April 23, 2015, 6:15 p.m.)
Review request for samza.
Bugs: SAMZA-657
https://issues.apache.org/jira/browse/SAMZA-657
Repository: samza
Description
-------
SAMZA-657.v1
1. Add the checkstyle xml files for the following packages (that have Java code)
samza-api
samza-core
samza-log4j
samza-kv
samza-test
2. Fix some coding style issues found by checkstyle.
3. Remove one class EpochPartitioner.java since it is from the old producer and
not used anywhere.
Some questions I have:
1. Current packaging hierarchy seems to me unnecessarily nested and hence the
import-control.xml rules quite messy. For example:
a. We have a "o.a.s.test" package, with nested "integration.join" etc, but the
test and test-util classes are acutally spread all over other packages, like
"o.a.s.system.mock"
b. We have a "o.a.s.serializers", and "o.a.s.logging.log4j.serializers". Shall
we just make one "serializers"?
c. We have both "o.a.s.serializers.model" and "o.a.s.job.model". Shall we just
make one "model"?
d. We have a top-level "o.a.s.task" and "o.a.s.container.grouper.task", etc..
Should we consider make the packaging hierarchy more clearer?
2. We are currently claiming to use 2 indentation in order to be consistent
with Scala, while there are some places that 4 indentation are also used. Shall
we just choose one standard and stick with it? The current checkstyle.xml
overrides default (4) to 2.
Diffs
-----
build.gradle 97de3a28f6379e3862eec845da87587b1d4f742e
checkstyle/checkstyle.xml PRE-CREATION
checkstyle/import-control.xml PRE-CREATION
samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java
092cb910b40d312217e86420bf1ddfbaf605e9e5
samza-api/src/main/java/org/apache/samza/config/Config.java
2b990506864c38ec2c46d55f27c2ba2f98f271ea
samza-api/src/main/java/org/apache/samza/config/MapConfig.java
38d7424429d4bf81614311c39630b165236d8fbb
samza-api/src/main/java/org/apache/samza/system/SystemStreamPartition.java
8dcea09ece60f91a890ff6b1abcb4e93c248dfe4
samza-api/src/main/java/org/apache/samza/util/BlockingEnvelopeMap.java
e30321d521f0fd7d3d69e2858352916142fb27bf
samza-api/src/main/java/org/apache/samza/util/SinglePartitionWithoutOffsetsSystemAdmin.java
01997ae22641b735cd452a0e89a49219e2874892
samza-api/src/test/java/org/apache/samza/config/TestConfig.java
d9f378d8de6da3bdf002e69dfb1e4605c3d90cec
samza-api/src/test/java/org/apache/samza/system/TestSystemStreamPartitionIterator.java
5af2a11812983cfb8b6a8a0927ef6f3eba7d340f
samza-api/src/test/java/org/apache/samza/util/TestBlockingEnvelopeMap.java
4eb87eb9033a45b5367480b9e38e18c629c79bc0
samza-core/src/main/java/org/apache/samza/serializers/model/SamzaObjectMapper.java
3517912eaafbf95f8c8cc70ab5869548a56b76e7
samza-core/src/main/scala/org/apache/samza/container/grouper/task/GroupByContainerCount.scala
8071fec3ac1bad76ddb3c6116e35c646be71d891
samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueIterator.java
2fb26e28685928547342b325ff6f0a63b3d83887
samza-kv/src/main/java/org/apache/samza/storage/kv/KeyValueStore.java
b708341abed15aaad34df5934f5f310bc1feb87a
samza-log4j/src/main/java/org/apache/samza/logging/log4j/StreamAppender.java
d3f25c0e03a727e64a774581384ef5aae9ef9c1c
samza-log4j/src/main/java/org/apache/samza/logging/log4j/serializers/LoggingEventStringSerde.java
8d8f5e8a8e5fd1e4d9e5482a5accd4a7ece463bc
samza-log4j/src/test/java/org/apache/samza/config/TestLog4jSystemConfig.java
6314a3ed7ae6d49328ba3f32af5d9d1097899009
samza-log4j/src/test/java/org/apache/samza/logging/log4j/TestJmxAppender.java
0bdade0d7c097bcb33acdfc8077c1ce57ad7988c
samza-test/src/main/java/org/apache/samza/system/mock/MockSystemAdmin.java
c0a20af5a2f4329ad4a2cff378ced3bececbc1cb
samza-test/src/main/java/org/apache/samza/test/integration/SimpleStatefulTask.java
67e56e0dd7b5cd2b636699f253e38c6265abf677
samza-test/src/main/java/org/apache/samza/test/integration/StatePerfTestTask.java
d7fecd83abdb1a171140cb13e17661a06e1d77c3
samza-test/src/main/java/org/apache/samza/test/integration/join/Checker.java
0598e51c158d2865ddd40685b9a3243561b4f556
samza-test/src/main/java/org/apache/samza/test/integration/join/Emitter.java
82a633d4e796cde347c4398d85e1903be735d067
samza-test/src/main/java/org/apache/samza/test/integration/join/EpochPartitioner.java
438d77c0d393727d3e6fde19a2706efd6ea1ce09
samza-test/src/main/java/org/apache/samza/test/integration/join/Joiner.java
d2c0c7eaf9c389e3f88b63a2eb7668b31d1b2daf
samza-test/src/main/java/org/apache/samza/test/integration/join/Watcher.java
7c82e0a7ac6b175cca935fc058a96aaade92fbe0
Diff: https://reviews.apache.org/r/33488/diff/
Testing
-------
./gradlew checkstyleMain checkstyleTest
Thanks,
Guozhang Wang
