----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33488/ -----------------------------------------------------------
Review request for samza. 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
