-
-
Notifications
You must be signed in to change notification settings - Fork 3
add armbian-desktop-cinnamon & armbian-desktop-i3-wm #26
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
WalkthroughThree new Armbian Desktop metapackages are added to debian/control: armbian-desktop-cinnamon, armbian-desktop-i3-wm, and armbian-desktop-kde. Each package is architecture-independent, marked as optional, provides armbian-desktop, and includes breaks/conflicts declarations against armbian-desktop. All three depend on armbian-desktop-common and their respective desktop environment packages (cinnamon-core or ubuntucinnamon-desktop-minimal for Cinnamon, i3-wm for I3, and kde-plasma-desktop for KDE). Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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 (2)
debian/control (2)
92-103: Consider adding Recommends for essential Cinnamon components.The package currently has minimal dependencies. While
ubuntucinnamon-desktop-minimalmay include necessary components, users installingcinnamon-core(the fallback option) might lack essential functionality. Consider adding a Recommends section similar to other desktop packages:
- Display manager (e.g.,
lightdmorslick-greeter)- File manager (e.g.,
nemo)- Terminal emulator (though covered by armbian-desktop-common)
- Cinnamon settings and applets
This would ensure a more complete desktop experience while maintaining the flexibility of a metapackage.
Depends: armbian-desktop-common, ubuntucinnamon-desktop-minimal|cinnamon-core, ${misc:Depends} +Recommends: + lightdm, + nemo, + cinnamon-control-center
106-117: Consider adding Recommends for i3 ecosystem components.While i3-wm users often prefer minimal setups, consider adding a Recommends section for essential i3 ecosystem tools and a display manager to ensure the window manager can actually start:
- Display manager (e.g.,
lightdm) - required to start X sessions- Status bar (e.g.,
i3statusori3blocks)- Application launcher (e.g.,
dmenuorrofi)- Lock screen (e.g.,
i3lock)This provides a working baseline while allowing experienced users to remove components they don't need.
Depends: armbian-desktop-common, i3-wm, ${misc:Depends} +Recommends: + lightdm, + i3status, + dmenu, + i3lock
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
debian/control(1 hunks)
🔇 Additional comments (2)
debian/control (2)
120-130: AI summary incorrectly claims KDE is new in this PR.The AI summary states "Three new Armbian Desktop metapackages are added" including armbian-desktop-kde, but the code annotations show the KDE package definition (lines 120-130) has no changes (no ~ markers). The PR title confirms only Cinnamon and I3-WM are being added.
101-101: No action required. The current packaging correctly uses Debian alternative dependencies (ubuntucinnamon-desktop-minimal|cinnamon-core) to handle cross-distro compatibility. On Debian systems lackingubuntucinnamon-desktop-minimal, apt will automatically resolve tocinnamon-core. This is standard Debian packaging practice.
|
either we do this, or remove them [whether one or both] from |
carve out from #25
add cinnamon and i3m DE packages. Not sure yet, if we really want to do that.