Conversation
Upgrade Nushell plugin dependencies from v0.111.0 to v0.112.0 and other system libraries including libc, nix, linux-raw-sys, and lscolors. Refactor the http-get-with-retry function in build.nu to use named flags instead of positional parameters and improve documentation with inline comments.
Updated lofty dependency from 0.23.3 to 0.24.0 in Cargo.lock. Simplified zip extraction logic in build.nu by removing unnecessary try-catch block and directly checking OS type for Windows-specific expansion. Enhanced check_and_download_prebuilt function to handle all combinations of optional parameters (filename, install-root, checksum-url) instead of only specific combinations, improving flexibility and coverage.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 41 minutes and 55 seconds. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates Nushell plugin dependencies (nu-plugin, nu-protocol to 0.112.0) and lofty decoder library (0.23 to 0.24) in Cargo.toml, increments the crate version to 0.2.4, and refactors build.nu to replace positional parameters with typed flags, improve error handling and validation paths, and update the package manifest version accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
Update multiple dependencies to their latest versions including cc, coreaudio-rs, doctest-file, pkg-config, rand, rand_core, rustc-hash, and semver. Remove older versions of anstream (0.6.21) and anstyle-parse (0.2.7) that are no longer needed, and consolidate dependency version specifications by removing explicit version pins where they're redundant.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.nu`:
- Around line 93-123: The code silently skips checksum verification when
fetching the checksum sidecar fails; change the branch that handles
$checksum_url so a failed HTTP fetch is treated as an error (fail-closed)
instead of returning null: in the computation of let expected (the
$expected_checksum / $checksum_url logic) replace the catch that returns null
with a catch that logs a warning (including the URL), cleans up the temporary
directory ($tmp_dir) and returns false (or otherwise aborts the install_prebuilt
flow), so that missing/failed checksum fetches do not cause an unverified binary
to be installed; keep the rest of the checksum comparison logic (is_md5,
actual/expected extraction) unchanged.
- Around line 279-282: The feature picker builds a comma-separated feature_list
from selected_features and passes it verbatim to cargo install
(--features=($feature_list)), but the picker options include invalid feature
names (e.g., flac, minimp3) that don't exist in Cargo.toml (which only defines
default, lite, all-decoders), causing cargo install to fail; fix by constraining
or mapping selections to the actual manifest features: update the selection
options used to populate selected_features (or add a validation/mapping step
after selected_features and before feature_list) so only "default", "lite", and
"all-decoders" are passed to cargo install, and ensure the log line (log info)
and cargo install invocation use the validated/mapped feature_list variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7e82282c-3779-4744-bfe5-9a94dba24cd7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlbuild.nunupm.nuon
Replace individual decoder feature flags with simplified generic options (default, lite, all-decoders) for cleaner dependency management. Add detailed error logging and cleanup when checksum download fails, improving robustness of the download_and_install function by handling network failures gracefully.
1 participant
This PR upgrades dependencies and fixes code quality issues in build.nu.
What changed
We updated Nushell plugin dependencies from 0.111.0 to 0.112.0, along with system libraries like libc, nix, linux-raw-sys, and lscolors. The build script was refactored to use named flags instead of positional parameters, making it clearer and easier to maintain.
Fixes
Dependencies
Incorporated the lofty 0.24 update from dependabot, which fixes FLAC metadata corruption and improves APE tag handling for multi-value items.
Testing
Build script tested on the current branch and handles all extraction formats correctly.
Summary by CodeRabbit
Release Notes
Chores
Bug Fixes