tballison commented on PR #3553:
URL: https://github.com/apache/tika/pull/3553#issuecomment-6693021880
Bot nits. Some sound legit.
```
1. Build break. forbiddenapis rejects String.format without a locale at
TarParserTest.java:95. Use
String.format(Locale.ROOT, ...). With that check skipped, all 59 module
tests pass.
2. The CB7 branch in ComicBookDetector is dead. I disabled the .cb7 name
and type checks, and Seven7ParserTest and
ComicBookFixturesTest still pass. That's because the 7z magic isn't
versioned, so the .cb7 glob or hint already
specializes it. The class javadoc itself says the detector is for "RAR
and tar". Only ComicBookDetectorTest pins that
branch. Drop it, which also makes the detector match what CHANGES and
the docs claim. POSIX tar doesn't need the
detector either, only GNU tar does, but that check is harmless.
3. ComicBook.isPage is inconsistent. It now splits names on \ as well as /
(for RAR4 names), but the __MACOSX/ exclusion
only matches the forward slash. Minor.
4. The tar cover pre-pass has an edge case. If the ParseContext supplies
an ArchiveStreamFactory with a non-default entry
encoding, TarFile (default encoding) and the stream return different
names, so the cover is silently lost. The pre-pass
also copies a streamed .cbt to a temp file and reads it a second time.
Both are low priority.
Hygiene
- TarParserTest: the new test was inserted between testEmbedded's javadoc
and its method, so that javadoc is now orphaned
(lines 48–51).
- RarParserTest: uses a fully qualified java.util.LinkedHashMap; import it
instead.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
