----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26542/#review56290 -----------------------------------------------------------
./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java <https://reviews.apache.org/r/26542/#comment96589> Small comment, should indent inside the if, else. ./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java <https://reviews.apache.org/r/26542/#comment96590> Same indentation comment. Trying to stay under 80 lines? ./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java <https://reviews.apache.org/r/26542/#comment96591> Would it be better to include this directly in ExternalParser? TesseractOCRParser has a very similar method. ./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java <https://reviews.apache.org/r/26542/#comment96592> Same comment, move this to ExternalParser? Make all external Parsers (e.g. OCR and GDAL) in a subpackage of o.a.t.parsers.external and make this method protected. ./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java <https://reviews.apache.org/r/26542/#comment96593> Should close the xhtml inside the finally, in case read throws an exception. ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java <https://reviews.apache.org/r/26542/#comment96594> Need an assumeTrue. Causes test to fail when gdalinfo not installed. ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java <https://reviews.apache.org/r/26542/#comment96595> Should remove the whitespace here. ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java <https://reviews.apache.org/r/26542/#comment96598> Need another assumeTrue(canRun()). ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java <https://reviews.apache.org/r/26542/#comment96596> Same, whitespace. ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java <https://reviews.apache.org/r/26542/#comment96597> Whitespace. - Tyler Palsulich On Oct. 11, 2014, 3:16 p.m., Chris Mattmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26542/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2014, 3:16 p.m.) > > > Review request for tika, Lewis McGibbney and Tyler Palsulich. > > > Bugs: TIKA-605 > https://issues.apache.org/jira/browse/TIKA-605 > > > Repository: tika > > > Description > ------- > > This is a patch that wraps the Geospatial Data Abstraction Library (GDAL) > tool that parses literally hundreds of geospatial formats as a Tika parser. > > > Diffs > ----- > > > ./trunk/tika-parsers/src/main/java/org/apache/tika/parser/gdal/GDALParser.java > PRE-CREATION > > ./trunk/tika-parsers/src/main/resources/META-INF/services/org.apache.tika.parser.Parser > 1629918 > > ./trunk/tika-parsers/src/test/java/org/apache/tika/parser/gdal/TestGDALParser.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/26542/diff/ > > > Testing > ------- > > Tested via unit tests, and ran locally. > > > Thanks, > > Chris Mattmann > >
