> On Aug. 11, 2014, 7:44 p.m., Mike Percy wrote: > > flume-ng-tests/src/test/java/org/apache/flume/test/util/SyslogAgent.java, > > line 22 > > <https://reviews.apache.org/r/24202/diff/4/?file=656309#file656309line22> > > > > This class needs documentation. > > Abraham Elmahrek wrote: > Could you be more specific? I'll add documentation at the class level for > sure.
Basically it goes back to the public vs. private thing... seemed weird to me that we would include a full test in the SyslogAgent and also provide public methods that no one outside SyslogAgent was calling. Seems to violate SRP. Since this is a test class, it is less critical, but it's also an infrastructure class, so worth thinking about. - Mike ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24202/#review50204 ----------------------------------------------------------- On Aug. 12, 2014, 1:52 a.m., Abraham Elmahrek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24202/ > ----------------------------------------------------------- > > (Updated Aug. 12, 2014, 1:52 a.m.) > > > Review request for Flume. > > > Bugs: FLUME-2438 > https://issues.apache.org/jira/browse/FLUME-2438 > > > Repository: flume-git > > > Description > ------- > > commit 321a62abc29ce495743b174cb8dec648e5448e2f > Author: Abraham Elmahrek <[email protected]> > Date: Fri Aug 1 18:33:11 2014 -0700 > > FLUME-2438 Make Syslog source message body configurable > > keepFields in the agent configuration can accept a space > separated list of fields that include either "timestamp" > or "hostname" or both. This will tell the agent to include > the specified fields in the message body. > > :100644 100644 427e0e3... 643bd67... M > flume-ng-core/src/main/java/org/apache/flume/source/MultiportSyslogTCPSource.java > :100644 100644 557d121... 996b31b... M > flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java > :100644 100644 985949c... a312cb6... M > flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java > :100644 100644 e84e4b6... 03e593c... M > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > :100644 100644 01b8905... 40a96a8... M > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > :100644 100644 a77bfc9... e966856... M > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > :100644 100644 9b97c8c... 72dc5c1... M > flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java > :100644 100644 2809163... 594e456... M > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java > :100644 100644 22fa200... d32e9cc... M > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogTcpSource.java > :100644 100644 95ee48c... 47f49d5... M > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > :100644 100644 82b7dd0... 5a4db9d... M > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java > :100644 100644 daf6e72... d179bf8... M flume-ng-doc/sphinx/FlumeUserGuide.rst > :000000 100644 0000000... dc463d5... A > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestMultiportSyslogTCPSource.java > :000000 100644 0000000... 8e50386... A > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestSyslogTCPSource.java > > > Diffs > ----- > > > flume-ng-core/src/main/java/org/apache/flume/source/MultiportSyslogTCPSource.java > 427e0e3 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogParser.java > 557d121 > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java > 985949c > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > e84e4b6 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 01b8905 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > a77bfc9 > > flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java > 9b97c8c > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java > 2809163 > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogTcpSource.java > 22fa200 > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > 95ee48c > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java > 82b7dd0 > flume-ng-doc/sphinx/FlumeUserGuide.rst daf6e72 > > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestSyslogSource.java > PRE-CREATION > flume-ng-tests/src/test/java/org/apache/flume/test/util/StagedInstall.java > a6bd5e9 > flume-ng-tests/src/test/java/org/apache/flume/test/util/SyslogAgent.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/24202/diff/ > > > Testing > ------- > > Added integration test and updated unit tests > > > Thanks, > > Abraham Elmahrek > >
