Skip to content
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

platform/3241: Add labels to artifact registry repositories #15

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

FabrizioCafolla
Copy link
Member

@FabrizioCafolla FabrizioCafolla commented Jan 15, 2025

PR Type

Enhancement


Description

  • Added support for labeling Artifact Registry repositories with both default and repository-specific labels
  • Introduced new default_labels variable with "managed-by: terraform" as default value
  • Added labels field to repository configuration to allow per-repository label customization
  • Updated documentation and changelog to reflect the new labeling functionality
  • Version bumped to 0.8.0 due to new feature addition

Changes walkthrough 📝

Relevant files
Enhancement
main.tf
Add labels support to artifact registry repositories         

main.tf

  • Added support for labels in artifact registry repositories by merging
    default_labels with repository-specific labels
  • +1/-0     
    Configuration changes
    variables.tf
    Add labels-related variables and configurations                   

    variables.tf

  • Added new labels field to repositories variable
  • Added new default_labels variable with default "managed-by: terraform"
    label
  • +9/-0     
    Documentation
    CHANGELOG.md
    Update changelog for version 0.8.0                                             

    CHANGELOG.md

    • Added entry for version 0.8.0 documenting labels feature addition
    +8/-0     
    README.md
    Update documentation with labels configuration                     

    README.md

  • Updated input variables documentation to include new labels-related
    parameters
  • Added reference to new data source for remote repository secrets
  • +3/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @sparkfabrik-ai-bot
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Validation Check

    The distinct() function used with merge() for labels should be validated to ensure it handles null or empty maps correctly to prevent potential runtime errors

    labels                 = distinct(merge(var.default_labels, each.value.labels))

    @sparkfabrik-ai-bot
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Remove unnecessary distinct() function when merging maps since merge() already ensures unique keys

    The distinct() function is not needed here since merge() already ensures unique
    key-value pairs. Remove it to simplify the code and improve readability.

    main.tf [66]

    -labels                 = distinct(merge(var.default_labels, each.value.labels))
    +labels                 = merge(var.default_labels, each.value.labels)
    Suggestion importance[1-10]: 5

    Why: The suggestion correctly identifies that distinct() is redundant since merge() already handles key uniqueness in maps. Removing it improves code clarity without affecting functionality.

    5

    @FabrizioCafolla FabrizioCafolla changed the title refs board#3241: Add labels to artifact registry repositories platform/3241: Add labels to artifact registry repositories Jan 15, 2025
    Copy link
    Contributor

    @Monska85 Monska85 left a comment

    Choose a reason for hiding this comment

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

    LGTM 👍

    @FabrizioCafolla FabrizioCafolla merged commit 780ede7 into main Jan 15, 2025
    1 check passed
    @FabrizioCafolla FabrizioCafolla deleted the feat/3241_add_labels branch January 15, 2025 10:41
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants