Skip to content

Conversation

@vic
Copy link
Owner

@vic vic commented Nov 11, 2025

No description provided.

@vic vic marked this pull request as ready for review November 11, 2025 14:33
Copilot AI review requested due to automatic review settings November 11, 2025 14:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves documentation on avoiding configuration duplication in default includes by adding a detailed explanatory comment with concrete examples. The changes also refactor existing code to follow the recommended pattern.

  • Adds comprehensive documentation explaining how to prevent config duplication when using fully parametric functions in defaults
  • Refactors hostname configuration to use den.lib.take.exactly for proper scoping
  • Simplifies vm-bootable and igloo.includes aspects by removing redundant hostname settings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

(if config ? _module.args.CI then den.aspects.ci-no-boot else { })

# NOTE: be cautious when adding fully parametric functions to defaults.
# defaults are included on EVERY host/user/home, and IF you are not careful
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Grammatical issue: "IF you are not careful" should use lowercase "if" since it's not at the beginning of a sentence and doesn't need emphasis. The capitalization is inconsistent with standard English writing conventions.

Suggested change
# defaults are included on EVERY host/user/home, and IF you are not careful
# defaults are included on EVERY host/user/home, and if you are not careful

Copilot uses AI. Check for mistakes.
@vic vic merged commit 374ad5f into main Nov 11, 2025
14 of 17 checks passed
@vic vic deleted the defaults branch November 11, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants