-
Notifications
You must be signed in to change notification settings - Fork 8
Make fh list releases
more granular
#200
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
WalkthroughBumps crate version and updates multiple Cargo dependency signatures (features/default-features and one removal); replaces CLI list row types with a Tabled-backed Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "cli::cmd::list"
participant Tabled as "tabled renderer"
participant Chrono as "chrono::DateTime<Utc>"
User->>CLI: run list command
CLI->>CLI: collect Release items (fields incl. optional datetimes)
CLI->>Tabled: pass Vec<Release>
Tabled->>Chrono: request formatting for Option<DateTime<Utc>>
Chrono-->>Tabled: formatted date or empty
Tabled-->>User: render table / JSON / CSV
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/cli/cmd/list.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (4)
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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/cli/cmd/list.rs (3)
91-105
: Ensure API field names match — otherwise deserialization will fail.
Release
now (de)serializes directly. If the FlakeHub API uses different casing (e.g., camelCase), add appropriate serde renames or a struct-level#[serde(rename_all = "...")]
.If you require RFC3339 for JSON, add per-field helpers, e.g.,
#[serde(with = "chrono::serde::ts_rfc3339::option")]
forpublished_at
/updated_at
. Based on learnings
107-114
: Optional: include timezone indicator in formatted timestamps.Current table output omits ‘Z/UTC’. Consider appending “Z” or using RFC3339 for clarity.
251-267
: Output routing is correct; verify default CSV headers align withRelease
fields
- JSON prints raw
Release
via serde.- TTY → table; non-TTY → CSV matches other commands.
- If header stability matters, confirm
csv::Writer
’s default header names/order.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml
(1 hunks)src/cli/cmd/list.rs
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cli/cmd/list.rs (1)
src/cli/cmd/mod.rs (2)
releases
(133-136)print_json
(295-299)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-artifacts (x86_64-darwin, macos-14-large, fh-X64-macOS)
- GitHub Check: build-artifacts (x86_64-linux, ubuntu-22.04, fh-ARM64-linux)
- GitHub Check: build-artifacts (aarch64-darwin, macos-latest-xlarge, fh-ARM64-macOS)
- GitHub Check: build-artifacts (aarch64-linux, namespace-profile-default-arm64, fh-X64-Linux)
- GitHub Check: rust-fmt-and-clippy
🔇 Additional comments (4)
Cargo.toml (2)
3-3
: Patch version bump looks good.No concerns.
11-14
: Enablingchrono
’sserde
is the right move for DateTime fields.Confirm downstream expects the chosen JSON date format (Chrono default). If a specific format is required (e.g., RFC3339), consider adding explicit serde helpers at the field sites.
Based on learnings
src/cli/cmd/list.rs (2)
1-1
: Chrono import aligns with new DateTime fields.Good addition.
115-120
: Helper looks fine.Returns dimmed text for Some and empty for None — OK.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cli/cmd/list.rs (1)
108-112
: Keep the timezone in release timestamps.Formatting with
%Y-%m-%d %H:%M
drops theUTC
suffix, so the table output looks like local time while the JSON still advertisesZ
. Keeping the zone avoids confusion.- Some(dt) => dt.format("%Y-%m-%d %H:%M").to_string(), + Some(dt) => dt.format("%Y-%m-%d %H:%M %Z").to_string(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lock
is excluded by!**/*.lock
flake.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml
(1 hunks)src/cli/cmd/list.rs
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cli/cmd/list.rs (1)
src/cli/cmd/mod.rs (2)
releases
(133-136)print_json
(295-299)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-artifacts (x86_64-linux, ubuntu-22.04, fh-ARM64-linux)
- GitHub Check: build-artifacts (x86_64-darwin, macos-14-large, fh-X64-macOS)
- GitHub Check: build-artifacts (aarch64-linux, namespace-profile-default-arm64, fh-X64-Linux)
- GitHub Check: rust-fmt-and-clippy
- GitHub Check: build-artifacts (aarch64-darwin, macos-latest-xlarge, fh-ARM64-macOS)
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cli/cmd/list.rs (1)
108-113
: Consider preserving timezone in formatted timestamps.
Line 110 formats timestamps as%Y-%m-%d %H:%M
, dropping seconds and the UTC designator. For global release data, including the timezone (e.g., viadt.to_rfc3339()
or%Z
) would make it clearer that the values are UTC.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lock
is excluded by!**/*.lock
flake.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml
(1 hunks)src/cli/cmd/list.rs
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cli/cmd/list.rs (1)
src/cli/cmd/mod.rs (2)
releases
(133-136)print_json
(295-299)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-artifacts (x86_64-darwin, macos-14-large, fh-X64-macOS)
- GitHub Check: build-artifacts (aarch64-linux, namespace-profile-default-arm64, fh-X64-Linux)
- GitHub Check: build-artifacts (x86_64-linux, ubuntu-22.04, fh-ARM64-linux)
- GitHub Check: build-artifacts (aarch64-darwin, macos-latest-xlarge, fh-ARM64-macOS)
- GitHub Check: rust-fmt-and-clippy
Currently this returns the bare minimum for a release:
Now it's more robust but without including almost always duplicated fields like
description
,readme
, andtotal
.Summary by CodeRabbit
New Features
Improvements
Chores