cleanup(localized_reader): drop unused seen HashSet (#190)#242
Closed
SAY-5 wants to merge 2 commits intoTrueNine:devfrom
Closed
cleanup(localized_reader): drop unused seen HashSet (#190)#242SAY-5 wants to merge 2 commits intoTrueNine:devfrom
seen HashSet (#190)#242SAY-5 wants to merge 2 commits intoTrueNine:devfrom
Conversation
Fix two CI failures from previous merge
Closes TrueNine#190. `read_flat_files` allocated a `HashSet<String>` and threaded it as `&mut seen` through `scan_directory` recursion just to call `seen.insert(full_name.clone())` once per new entry. The set was never read for dedup — `entries.iter_mut().find(|e| e.name == full_name)` is the actual existence check on the line above (TrueNine#191 tracks the O(n²) → HashMap follow-up; this PR is just the dead write). Removing the parameter: - Deletes a per-call `String::clone` (set insertion took ownership of a clone of the name we already used to construct the entry). - Drops the `HashSet` allocation on every call to `read_flat_files`. - Removes the `use std::collections::HashSet` import. - Simplifies `scan_directory`'s signature (4 args → 3). `cargo build` + `cargo test --lib repositories` clean (45/45 repository tests pass). `cargo clippy --lib` shows no new warnings on `localized_reader.rs`.
2 tasks
Owner
|
Merged into dev via cherry-pick. The PR base was changed from main to dev, which caused conflicts because dev had diverged significantly. All commits have been cherry-picked onto dev and pushed. Thanks for the contributions! |
Owner
|
Thank you for the contribution! All commits have been cherry-picked and merged into the dev branch. 🙏 |
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.
Closes #190.
read_flat_filesallocated aHashSet<String>and threaded it as&mut seenthroughscan_directoryrecursion just to callseen.insert(full_name.clone())once per new entry. The set was never read for dedup —entries.iter_mut().find(|e| e.name == full_name)is the actual existence check on the line above. (#191 tracks the O(n²) → HashMap follow-up; this PR is just the dead write.)Removing the parameter:
String::clone(set insertion took ownership of a clone of the name we already used to construct the entry).HashSetallocation on every call toread_flat_files.use std::collections::HashSetimport.scan_directory's signature (4 args → 3).Test plan
cargo build --manifest-path sdk/Cargo.toml— cleancargo test --manifest-path sdk/Cargo.toml --lib repositories— 45/45 repository tests passcargo clippy --manifest-path sdk/Cargo.toml --lib— no new warnings onlocalized_reader.rs