chore: add codeowners#9
Merged
Merged
Conversation
antouhou
approved these changes
Nov 4, 2021
5 tasks
5 tasks
QuantumExplorer
added a commit
that referenced
this pull request
May 19, 2026
… into helpers Closes thepastaclaw review nitpick #9 (240c820 review, carried forward from earlier rounds) — the `match &primary_key_sum_property { Some(prop) => ItemWithSumItem... }` pattern was open-coded across all five insert arms in `add_document_to_primary_storage_0` (keep-history × 4 + insert_without_check × 4 + trailing else × 4 document_info shapes), and the parallel `required_item_with_sum_item_space` vs `required_item_space` size branch was duplicated three times in the `DocumentEstimatedAverageSize` cost arms. The 6c59b93 fix that wired `DocumentEstimatedAverageSize` for sum-aware sizes had to be replicated independently across the two relevant branches because there was no helper — exactly the drift risk the reviewer called out. - `build_primary_element(document, serialized, flags, sum_property)` centralizes the Item / ItemWithSumItem dispatch and the `read_document_sum_contribution` call. Returns `Result<Element>` so the contract-level invariant violation (i64 conversion fail) surfaces as `CorruptedCodeExecution` from one place. - `required_primary_element_space(max_size, sum_property, grove_version)` is its cost-only twin for `DocumentEstimatedAverageSize` arms. The docstring spells out that the two helpers must stay in lock-step or estimation and execution drift on summable inserts. - Replaced 9 element-construction sites + 3 size-estimation sites with single-line helper calls. Net -42 LOC despite adding two fully-documented helpers above the impl block. - Trimmed the now-redundant header comment paragraph that explained the Element::Item -> ItemWithSumItem replacement inline; the helpers' docstrings carry the same contract. Validation: cargo check + cargo clippy -p drive --lib clean; all 3174 drive lib tests pass (refactor is behavior-identical, only collapses duplication). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Add initial codeowners
What was done?
How Has This Been Tested?
None
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only