Redesign wings startup banner - #204
Conversation
Replace the plain boot banner with an ANSI Shadow "PELICAN" wordmark rendered in a vertical cyan->blue gradient, followed by a WINGS rule with the build version centered beneath it, a tagline, aligned Source/Docs link rows, and a star-the-project call-to-action. Color depth adapts to the terminal: 24-bit truecolor when COLORTERM advertises it, a 256-color approximation otherwise, and no escape codes at all when stdout is not a TTY. The runtime copyright/MIT block is dropped (still shipped in the LICENSE file), and the log-path notice now prints below the banner.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughStartup output now reports the configured log path directly. The previous logo and license text were replaced with a terminal-aware Pelican banner that supports multiple color modes and includes version, links, and a GitHub star call-to-action. ChangesStartup output
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This PR changes the startup banner presentation only; no actionable merge-blocking risk remains, so it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/root.go`:
- Around line 487-496: Update detectBannerColor to return bannerPlain before the
COLORTERM switch when TERM is "dumb" or the NO_COLOR environment variable is
set. Preserve the existing stdout character-device check and color selection for
other terminals.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 41cfbd27-60eb-4367-bfe6-ca3aaa274fed
📒 Files selected for processing (1)
cmd/root.go
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Analyze (go)
- GitHub Check: Build and Test (ubuntu-22.04, 1.25.12, linux, arm64)
- GitHub Check: Build and Test (ubuntu-22.04, 1.26.5, linux, arm64)
- GitHub Check: Test macOS (1.25.12)
- GitHub Check: Build and Test (ubuntu-22.04, 1.25.12, linux, amd64)
- GitHub Check: Build and Test (ubuntu-22.04, 1.26.5, linux, amd64)
- GitHub Check: Test macOS (1.26.5)
🔇 Additional comments (3)
cmd/root.go (3)
118-118: LGTM!
516-543: LGTM!
545-609: LGTM!
| func detectBannerColor() bannerColor { | ||
| fi, err := os.Stdout.Stat() | ||
| if err != nil || fi.Mode()&os.ModeCharDevice == 0 { | ||
| return bannerPlain | ||
| } | ||
| switch os.Getenv("COLORTERM") { | ||
| case "truecolor", "24bit": | ||
| return bannerTrue | ||
| } | ||
| return banner256 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return plain output when color is disabled.
A character device does not guarantee 256-color support. TERM=dumb terminals receive literal ANSI escape sequences from this branch. Users who set NO_COLOR also receive color output.
Return bannerPlain before the color-mode switch when TERM is "dumb" or NO_COLOR is set.
Proposed fix
func detectBannerColor() bannerColor {
fi, err := os.Stdout.Stat()
if err != nil || fi.Mode()&os.ModeCharDevice == 0 {
return bannerPlain
}
+ if os.Getenv("TERM") == "dumb" || os.Getenv("NO_COLOR") != "" {
+ return bannerPlain
+ }
switch os.Getenv("COLORTERM") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func detectBannerColor() bannerColor { | |
| fi, err := os.Stdout.Stat() | |
| if err != nil || fi.Mode()&os.ModeCharDevice == 0 { | |
| return bannerPlain | |
| } | |
| switch os.Getenv("COLORTERM") { | |
| case "truecolor", "24bit": | |
| return bannerTrue | |
| } | |
| return banner256 | |
| func detectBannerColor() bannerColor { | |
| fi, err := os.Stdout.Stat() | |
| if err != nil || fi.Mode()&os.ModeCharDevice == 0 { | |
| return bannerPlain | |
| } | |
| if os.Getenv("TERM") == "dumb" || os.Getenv("NO_COLOR") != "" { | |
| return bannerPlain | |
| } | |
| switch os.Getenv("COLORTERM") { | |
| case "truecolor", "24bit": | |
| return bannerTrue | |
| } | |
| return banner256 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/root.go` around lines 487 - 496, Update detectBannerColor to return
bannerPlain before the COLORTERM switch when TERM is "dumb" or the NO_COLOR
environment variable is set. Preserve the existing stdout character-device check
and color selection for other terminals.
|
I kinda liked the big @lancepioch you have conflicts now. please fix those. |
3 participants
Summary by CodeRabbit
Style
Logging
wings.logpath directly.