tballison commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6044776267
2. y, i get it.
Robot's latest. Please push back if the robot is just finding stuff because
I asked it to. Some of these sound reasonable to me, but you know this area of
the code better than I do. :rofl:
```
Remaining maintainer decision
- onenote:entityGuids is a catch-all for any JCID the switch doesn't name.
It is empty on every fixture, and a frozen key
with no known producer is pure surface. I'd drop it, or at minimum give
every new key a one-line javadoc naming the
source object (0x30 page metadata, 0x0B page node, header guidFile vs
FileIdentityGuid) so the semantics freeze with the
name.
- OneNoteJcid, OneNoteGuidCollector, and GUID.fromMicrosoftBytes are new
public classes/methods. Public is the norm for
parser internals in Tika, so fine unless you want them package-private
with the fsshttpb subpackage moved.
Edge case / usability (punt-able)
- Classic files have no <div class="page"> at all, so page GUIDs can't be
tied to page text there. A follow-on JIRA: emit
a page div per jcidPageNode in the classic walker.
Hygiene
- findPageGuid(cell) duplicates the root-resolution logic of walkCell
(unresolvedRoot/resolvedContentRoot). Share one
"roots to walk" helper so they can't diverge.
- testGuidsCollectedBeforeWalkFailurePublished relies on a
NullPointerException from a null objectGroups. A null guard
later would break the test for the wrong reason. Override
indexObjectsById to throw instead.
- MSOneStorePackageTest repeats the same 16-byte literal eight-plus times;
one helper would cut the file substantially.
- Bags are lexically sorted, so document order of pages is lost. The div
ids recover it for FSSHTTPB only. Fine as is, but
a deliberate choice.
```
--
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]
