Added new packages and cleaned up the var files for Ansible#34
Added new packages and cleaned up the var files for Ansible#34
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Ansible vars structure to eliminate package duplication and improve maintainability by introducing shared packages common across configurations. It centralizes approximately 60 lines of duplicated package definitions into a shared section while maintaining configuration-specific customizations.
- Introduced
shared_packagescontaining common packages for all configurations - Streamlined
config_packagesto only contain configuration-specific additions - Enhanced task validation and error handling with proper config validation and package merging
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
defilan-osconfig/roles/defilan-osconfig/vars/main.yml |
Restructured package organization with shared packages and streamlined config-specific packages |
defilan-osconfig/roles/defilan-osconfig/tasks/main.yml |
Added config validation, package merging logic, and improved error handling |
README.md |
Updated documentation with comprehensive package lists organized by configuration type |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - iterm2 | ||
| - visual-studio-code | ||
| - 1password | ||
| - rectangle |
There was a problem hiding this comment.
The 'lightweight' configuration overrides shared packages by redefining the same packages (1password, arc, istat-menus, iterm2, rectangle). This defeats the purpose of the shared_packages refactor. Consider removing these duplicated packages from the lightweight config since they're already in shared_packages.
| - name: Set config variable with default | ||
| set_fact: | ||
| config: "{{ lookup('env', 'config') }}" | ||
| config: "{{ config | default('home') }}" |
There was a problem hiding this comment.
This line overwrites any existing 'config' variable with the default value if it's undefined. However, if 'config' is already defined (e.g., from environment or playbook vars), the 'default' filter won't apply and this task becomes redundant. Consider using 'when: config is not defined' condition or checking if the variable exists first.
| config: "{{ config | default('home') }}" | |
| config: "{{ config | default('home') }}" | |
| when: config is not defined |
🚀 Refactor Ansible Package Organization for Better Maintainability
Summary
This PR refactors the Ansible vars file to eliminate package duplication and improve maintainability by introducing a hierarchical package organization system.
�� Changes Made
Package Structure Refactor
developerandhomeconfigurationsshared_packagessection containing all packages common to both developer and home configsconfig_packagesnow only contains unique additions for each configurationPackage Organization
aws/tap,hashicorp/tap,twilio/brewneovim,tmux,git-extras,go,kubectl,terraform,node,redis, etc.arc,iterm2,visual-studio-code,1password,docker,postman, etc.Tasks File Improvements
combinefilter to merge shared and config-specific packagesDocumentation Updates
🎯 Benefits
shared_packagesonce, available in both dev and home configs🧪 Testing
📝 Configuration Options
1password,arc,istat-menus,iterm2,rectangle)discord,gimp,krisp,microsoft-office,nordvpn,signal)This refactor makes the dotfiles much more maintainable while preserving all existing functionality and improving the user experience.