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




flume-ng-doc/sphinx/FlumeUserGuide.rst (line 1137)
<https://reviews.apache.org/r/50134/#comment208183>

    I think `fileHeader` would be a better name for this to be consistent with 
the `SpoolDirectorySource`.
    And speaking of consistency it might be worth to add `fileHeaderKey` too.



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java
 (line 67)
<https://reviews.apache.org/r/50134/#comment208190>

    line is longer than 100 characters



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java
 (line 198)
<https://reviews.apache.org/r/50134/#comment208184>

    - formatting: a space is missing after if
    - also please remove the unnecessary `== true`
    - the redundant parentheses around `headers != null && !headers.isEmpty()` 
can be removed too, I'm not sure whether it makes it easier to understand



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java
 (line 200)
<https://reviews.apache.org/r/50134/#comment208185>

    unnecessary extra pair of parenthesis



flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java
 (line 204)
<https://reviews.apache.org/r/50134/#comment208189>

    line is too long (107 chars)



flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
 (line 43)
<https://reviews.apache.org/r/50134/#comment208191>

    please remove `.*` import



flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
 (lines 320 - 322)
<https://reviews.apache.org/r/50134/#comment208188>

    it'd be better to use `{f1,f2,f3}.getAbsolutePath()` instead of 
`tmpDir.getAbsolutePath()+"/file{1,2,3}"`


- Denes Arvay


On July 18, 2016, 1:49 p.m., qiao wen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50134/
> -----------------------------------------------------------
> 
> (Updated July 18, 2016, 1:49 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2955
>     https://issues.apache.org/jira/browse/FLUME-2955
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Log file path is necessary to locate the log. Add a parameter PATH_HEADER in 
> the TaildirSourceConstants. If the parameter is true, the file path will be 
> added to the header of flume event. Defaut value is false.
> 
> 
> Diffs
> -----
> 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst f9ca1b2 
>   
> flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/ReliableTaildirEventReader.java
>  1409f25 
>   
> flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSource.java
>  eae1b1a 
>   
> flume-ng-sources/flume-taildir-source/src/main/java/org/apache/flume/source/taildir/TaildirSourceConfigurationConstants.java
>  2c49540 
>   
> flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
>  e090b74 
> 
> Diff: https://reviews.apache.org/r/50134/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install -DskipTests -> built
> junit tests for flume-taildir-source module -> passed
> 
> 
> Thanks,
> 
> qiao wen
> 
>

Reply via email to