chore: sync from monorepo @9b81d97#25
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 51 minutes and 38 seconds.Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces several regressions, including the removal of essential configuration files like .gitignore and osv-scanner.toml, and the deletion of Clippy lint suppressions which will likely cause CI failures. The changes also replace idiomatic Rust patterns, such as range checks using .contains() and derived Default implementations, with less standard and more verbose code. Additionally, dependencies in Cargo.lock were downgraded, and documentation updates introduced non-ASCII characters that may affect searchability. Feedback focuses on restoring configurations and reverting to idiomatic Rust practices to maintain code quality and build stability.
I am having trouble creating individual review comments. Click here to see my feedback.
.gitignore (1-6)
Removing the .gitignore file is a significant regression. It will cause build artifacts and environment-specific files (like target/, node_modules/, and .DS_Store) to be tracked by Git, leading to repository bloat and potential conflicts. If this is a standalone repository, this file must be retained.
osv-scanner.toml (1-58)
Removing the osv-scanner.toml configuration is a security regression. This file documented the rationale for ignoring specific vulnerabilities that are currently unfixable due to upstream dependencies. Without this file, security audits become more difficult, and automated scanners may produce noise or fail to enforce the project's security policy.
resq-airspace/src/lib.rs (1)
The removal of clippy::too_many_arguments and clippy::diverging_sub_expression allows will likely cause CI failures. The project contains functions with many arguments (e.g., initialize_property::handler with 9 arguments) and uses macros like err! that can trigger the diverging sub-expression lint. These attributes should be retained to maintain a clean linting state.
#![allow(
unexpected_cfgs,
clippy::too_many_arguments,
clippy::diverging_sub_expression
)]resq-airspace/src/instructions/initialize_property.rs (55)
The #[allow(clippy::too_many_arguments)] attribute was removed, but the handler function still takes 9 arguments. This exceeds Clippy's default threshold of 7 and will cause linting failures in CI. Unless the arguments are refactored into a struct, this attribute should be restored.
#[allow(clippy::too_many_arguments)]
pub fn handler(
resq-airspace/src/instructions/initialize_property.rs (69)
Using explicit comparisons for range validation is less idiomatic than using .contains(). The previous version was more readable and standard for Rust.
(1..=8).contains(&vertex_count),
resq-airspace/src/instructions/record_crossing.rs (65-66)
The documentation now uses the non-ASCII minus sign − (U+2212) instead of the standard hyphen-minus -. This is inconsistent with the actual code (which uses ASCII -) and may cause issues with searchability or automated tools that expect ASCII characters for numeric ranges.
resq-airspace/src/instructions/record_crossing.rs (98)
Using explicit comparisons for range validation is less idiomatic than using .contains(). Reverting to the range-based check improves readability.
(-900_000_000..=900_000_000).contains(&lat),
resq-airspace/src/instructions/record_crossing.rs (102)
Using explicit comparisons for range validation is less idiomatic than using .contains(). Reverting to the range-based check improves readability.
(-1_800_000_000..=1_800_000_000).contains(&lon),
resq-delivery/src/instructions/record_delivery.rs (83)
Using explicit comparisons for range validation is less idiomatic than using .contains(). Reverting to the range-based check improves readability.
(-900_000_000..=900_000_000).contains(&lat),
resq-delivery/src/instructions/record_delivery.rs (87)
Using explicit comparisons for range validation is less idiomatic than using .contains(). Reverting to the range-based check improves readability.
(-1_800_000_000..=1_800_000_000).contains(&lon),
resq-airspace/src/state/airspace_account.rs (20)
The removal of the derived Default implementation in favor of a manual one is a regression in code conciseness. Modern Rust supports #[derive(Default)] with the #[default] attribute on enum variants, which is the preferred approach.
#[derive(AnchorSerialize, AnchorDeserialize, Clone, Copy, PartialEq, Eq, Debug, Default)]
resq-airspace/src/state/airspace_account.rs (32-36)
This manual Default implementation is redundant if #[derive(Default)] and #[default] are used on the enum definition. Reverting to the derived version is more idiomatic.
Automated sync from resq-software/resQ@
9b81d97.Review before merging — direct pushes to standalone repos are preserved.