-
Notifications
You must be signed in to change notification settings - Fork 273
Some small Labrinth refactors #3698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Minor refactors across Labrinth routes and models to streamline logic and improve code clarity
- Replaced inline array cloning with
std::slice::from_ref
in version file endpoints - Rewrote
capitalize_first
to use a char iterator instead of in-place string mutation - Removed custom duplicate-removal function in project model in favor of a
HashSet
+collect_vec
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
apps/labrinth/src/routes/v3/version_file.rs | Swapped &[hash.clone()] for std::slice::from_ref(&hash) in multiple endpoints |
apps/labrinth/src/routes/v2_reroute.rs | Updated capitalize_first to iterate over chars with a boolean flag |
apps/labrinth/src/models/v3/projects.rs | Dropped remove_duplicates ; used HashSet + collect_vec for deduplication; fixed typos |
Comments suppressed due to low confidence (3)
apps/labrinth/src/routes/v3/version_file.rs:55
- [nitpick] The algorithm-resolution logic is duplicated across several handlers; consider extracting it into a helper function (e.g.,
resolve_algorithm(&hash_query, &hash)
) to reduce repetition.
let algorithm = hash_query.algorithm.clone().unwrap_or_else(|| { default_algorithm_from_hashes(std::slice::from_ref(&hash)) });
apps/labrinth/src/routes/v2_reroute.rs:270
- [nitpick] Avoid using
char
as a variable name since it shadows the primitive type; consider renaming it toc
orch
for clarity.
.map(|char| {
apps/labrinth/src/models/v3/projects.rs:127
- Using a
HashSet
for deduplication can scramble element order. To preserve the original sequence while removing duplicates, consider usingmem::take(v).into_iter().unique().collect::<Vec<_>>()
.
*v = HashSet::<_, RandomState>::from_iter(mem::take(v).into_iter()).into_iter().collect_vec();
This addresses an unintended behavior change on 157647f.
I wonder why we don't run these more often...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this stuff is over my head, so it might be better to wait for Josiah's review, but I'm not seeing anything problematic here
This PR proposes some small and minor refactors to Labrinth code I did while executing other development tasks.