feat: add Distribution and DistributionFamily enums - #65
Conversation
Add centralized distribution configuration with support for Ubuntu, Debian, Fedora, CentOS, Rocky, Alma, RHEL, and Amazon Linux distributions. Includes display names, family grouping, and utility methods for playbook integration.
WalkthroughAdds two enums (Distribution, DistributionFamily), replaces ad-hoc distro checks with enum-driven resolution across commands and services, and enhances the server-info playbook to detect distribution families and emit a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Cmd as ServerInfoCommand
participant Play as playbooks/server-info.sh
participant EnumD as Distribution
participant EnumF as DistributionFamily
User->>Cmd: run server:info
Cmd->>Play: execute playbook
Play->>Play: detect distro (ID from /etc/os-release)
Play->>Play: normalize distro slug
Play->>Play: detect_family(distro) %% new family detection
Play->>Play: ensure_tools(family, perms) %% ensure by family
Play-->>Cmd: YAML { distro:..., family:..., perms:... }
Cmd->>EnumD: Distribution::tryFrom(distro_slug)
alt found
EnumD-->>Cmd: enum case
Cmd->>EnumD: displayName()
EnumD-->>Cmd: human-readable name
else not found
Cmd-->>User: "Unknown"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/Enums/DistributionFamily.php (2)
22-25: Consider deferring the playbookName() method until it's needed.The
playbookName()method is not called anywhere in the provided code. Per the coding guidelines to eliminate single-use methods and follow YAGNI principles, consider adding this utility method when it's actually needed for playbook integration.As per coding guidelines
32-35: Consider deferring the names() method until it's needed.The
names()static method is not used anywhere in the provided code. Consider adding this utility method when a use case arises, keeping the enum focused on currently required functionality.As per coding guidelines
app/Enums/Distribution.php (1)
43-51: Consider deferring the family() method until it's needed.The
family()method is not called anywhere in the provided code. While the Distribution enum itself is essential, this particular method could be added later when family-based logic is actually needed in the PHP codebase.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/Console/Server/ServerInfoCommand.php(2 hunks)app/Console/Server/ServerProvisionDigitalOceanCommand.php(1 hunks)app/Enums/Distribution.php(1 hunks)app/Enums/DistributionFamily.php(1 hunks)app/Services/DigitalOcean/DigitalOceanAccountService.php(4 hunks)app/Traits/PlaybooksTrait.php(3 hunks)playbooks/server-info.sh(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Services/DigitalOcean/DigitalOceanAccountService.phpapp/Enums/Distribution.phpapp/Console/Server/ServerProvisionDigitalOceanCommand.phpapp/Enums/DistributionFamily.phpapp/Console/Server/ServerInfoCommand.phpapp/Traits/PlaybooksTrait.php
🧬 Code graph analysis (3)
app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
app/Enums/Distribution.php (1)
slugs(58-61)
app/Enums/DistributionFamily.php (1)
app/Enums/Distribution.php (1)
family(43-51)
app/Traits/PlaybooksTrait.php (1)
app/Services/IOService.php (1)
writeln(458-464)
🔇 Additional comments (13)
app/Traits/PlaybooksTrait.php (3)
35-40: LGTM! Clear documentation for environment variables.The documentation clearly describes the standard playbook environment variables and their purposes.
83-86: LGTM! Improved UX for streaming output.Adding a visible header before streaming output helps users understand what's happening in real-time.
53-53: No issues found—the path calculation change is correct and verified.The change from
dirname(__DIR__, 3)todirname(__DIR__, 2)is the proper fix. With the file located atapp/Traits/PlaybooksTrait.php,dirname(__DIR__, 2)correctly resolves to the project root, which is then used to construct the playbooks path. The playbooks directory exists at the project root, and the implementation is sound.app/Console/Server/ServerProvisionDigitalOceanCommand.php (1)
284-284: LGTM! Updated hint aligns with expanded distribution support.The hint text now accurately reflects the broader distribution support introduced by the Distribution enum.
app/Console/Server/ServerInfoCommand.php (1)
115-118: LGTM! Clean enum-based distribution resolution.The refactoring from static mapping to enum-based resolution using
Distribution::tryFrom()anddisplayName()is well-implemented, with proper nullable chaining and fallback handling.app/Services/DigitalOcean/DigitalOceanAccountService.php (2)
104-104: LGTM! Cleaner filtering using enum-based distribution slugs.Replacing the hard-coded array with
Distribution::slugs()centralizes the distribution configuration and makes it easier to maintain.
113-114: LGTM! Sorting improves user experience.Adding
asort()ensures image options are presented in a predictable, alphabetical order.app/Enums/Distribution.php (2)
26-38: LGTM! Display names are actively used.The
displayName()method is used by ServerInfoCommand to show human-readable distribution names. The implementation is clean and comprehensive.
58-61: LGTM! The slugs() method is actively used.The
slugs()static method is used by DigitalOceanAccountService for image filtering, providing a centralized source of supported distribution identifiers.playbooks/server-info.sh (4)
39-44: LGTM! Proper distribution normalization.The normalization logic correctly handles common distribution identifier variations (almalinux→alma, rockylinux→rocky, amzn→amazon), ensuring consistent output that aligns with the Distribution enum values.
68-88: LGTM! Well-structured family detection.The
detect_family()function correctly groups distributions into families, matching the logic in the PHP Distribution enum's family() method. The mapping is clear and comprehensive.
122-138: LGTM! Improved abstraction by operating on family level.Changing
ensure_tools()to accept family instead of distro is cleaner, as distributions within the same family share package managers. The combined case forfedora|redhat|amazonwith yum/dnf fallback correctly handles the various RHEL-like distributions.
186-207: LGTM! Family field enhances output while maintaining backward compatibility.Adding the family field to the YAML output provides valuable information for downstream processing while preserving the existing distro field. This aligns well with the enum-based approach introduced in the PHP code.
Removed playbookName() and names() from DistributionFamily, and family() from Distribution. These methods had no current usage and will be added back when actually needed per YAGNI principles.
Add centralized distribution configuration with support for Ubuntu, Debian, Fedora, CentOS, Rocky, Alma, RHEL, and Amazon Linux distributions. Includes display names, family grouping, and utility methods for playbook integration.
Summary by CodeRabbit
New Features
Improvements