feat(ledger): fctl ledger logs list #28
Conversation
Dav-14
force-pushed
the
feat/pla-140-featfctl-add-command-for-get-ledger-logs
branch
from
f7438f7 to
a7954d2
Compare
|
Warning Rate limit exceeded@Dav-14 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 10 seconds before requesting another review. ⌛ 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe pull request introduces logging functionality into the ledger CLI application. A new logs package is added with a file that implements log listing capabilities. This includes new types for managing state (pagination, time filtering) and a set of commands to list and manage logs. Additionally, the primary ledger command is updated to integrate the new logs command into its hierarchy, ensuring that users can interact with log management via the command-line interface. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as Command Line
participant LC as ListController
participant API as Ledger API
participant PT as pterm Renderer
U->>CLI: Execute "ledger logs list"
CLI->>LC: Invoke Run() with parameters (pagination, time filters)
LC->>API: Request logs with specified parameters
API-->>LC: Return logs data and updated cursor
LC->>PT: Call Render() to format logs output
PT-->>CLI: Display formatted log table to user
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Dav-14
force-pushed
the
feat/pla-140-featfctl-add-command-for-get-ledger-logs
branch
from
a7954d2 to
45f6b9c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/ledger/logs/list.go (3)
17-24: Fix typo in flag name constant.There's a typo in the
endTimeFLagconstant name (capital 'L').- endTimeFLag = "end-time" + endTimeFlag = "end-time"
65-101: Add validation for time range.The time parsing logic is correct, but there's no validation to ensure that start time is before end time when both are provided.
if endTime := fctl.GetString(cmd, endTimeFLag); endTime != "" { str, err := time.Parse(timeRFC, endTime) if err != nil { return nil, fmt.Errorf(`invalid end time parsing with %s: %w`, timeRFC, err) } request.EndTime = &str + if request.StartTime != nil && !request.StartTime.Before(str) { + return nil, fmt.Errorf("start time must be before end time") + } }
103-142: Consider enhancing table output.The table rendering is functional but could be improved for better readability.
- Consider adding column alignment for better readability:
return pterm.DefaultTable. WithHasHeader(). WithWriter(cmd.OutOrStdout()). WithData(tableData). + WithRightAlignment(). Render()
- Consider adding color coding for different log types to improve visual distinction:
return []string{ log.Date.Format(timeRFC), strconv.FormatInt(log.ID, 10), - string(log.Type), + pterm.LightYellow(string(log.Type)), log.Hash, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/ledger/logs/list.go(1 hunks)cmd/ledger/logs/root.go(1 hunks)cmd/ledger/root.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Tests
🔇 Additional comments (4)
cmd/ledger/logs/root.go (1)
8-16: LGTM! Well-structured command setup.The command is well-organized with clear aliases and description. The integration with the list command is properly handled.
cmd/ledger/root.go (1)
6-6: LGTM! Clean integration of logs command.The logs command is properly imported and integrated into the ledger command hierarchy.
Also applies to: 28-28
cmd/ledger/logs/list.go (2)
26-44: LGTM! Clean store and controller implementation.The store and controller structures are well-defined with clear separation of concerns.
46-59: LGTM! Well-structured command with clear flags.The command setup is comprehensive with appropriate flags for pagination and time filtering.
Uh oh!
There was an error while loading. Please reload this page.