feat: enhance Rollytics service output and configuration#204
Conversation
- Added styled output for Rollytics service address and Swagger documentation URL in the command line interface. - Introduced a new constant for the default indexer address in the Minitia model. - Updated ScanPayload to include an indexer field for better data handling. - Modified the terminal state view to inform users about the indexer status and its importance for data population. - Changed Docker Compose port mapping from 8088 to 6767 for the Rollytics service.
WalkthroughAdds indexer support and rollytics port/styling: new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Launcher (CLI)
participant R as Rollytics Service
participant I as Indexer (Minitia)
participant Docker as Docker Compose
CLI->>Docker: create/start services (rollytics-api mapped 6767:8080)
Docker-->>R: rollytics starts
R-->>CLI: startup complete + styled URLs printed
CLI->>I: launch scan payload (includes Indexer = http://localhost:6767)
Note over CLI,I: Data becomes visible once indexer service is running
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@models/minitia/launch.go`:
- Around line 3199-3205: Replace the awkward phrase in the return statement
where fmt.Sprintf builds the message (inside the styles.Text(...) used for the
Chrome recommendation) with clearer wording; locate the return that calls
m.WrapView(state.weave.Render()) + state.scanLink + fmt.Sprintf(...) and update
the third argument string (currently "Open this in Chrome is recommended
because...") to something concise like "Opening this in Chrome is recommended
because some browsers may not support localhost access from a different host;
alternatively, adjust your browser settings to allow it." Keep the rest of the
message (the indexer warning using "weave rollup indexer start") unchanged.
🧹 Nitpick comments (1)
cmd/rollytics.go (1)
55-56: Avoid duplicating the rollytics base URL literal.
Introduce a local constant to keep the two lines in sync and reduce future drift.♻️ Suggested refactor
- fmt.Println("Rollytics Address: ", styles.Text("http://localhost:6767", styles.Cyan)) - fmt.Println("Swagger Docs: ", styles.Text("http://localhost:6767/swagger", styles.Cyan)) + const rollyticsBaseURL = "http://localhost:6767" + fmt.Println("Rollytics Address: ", styles.Text(rollyticsBaseURL, styles.Cyan)) + fmt.Println("Swagger Docs: ", styles.Text(rollyticsBaseURL+"/swagger", styles.Cyan))
- Updated the message in the terminal state view to recommend opening in Chrome for better compatibility with localhost access. - Enhanced clarity by rephrasing the guidance on browser settings adjustments.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Changes