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.

Sign up for GitHub

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

Jump to bottom

refactor: common config variable #6

Merged
merged 28 commits into from
Mar 1, 2024
Merged

refactor: common config variable #6

merged 28 commits into from
Mar 1, 2024

Conversation

FoSix
Copy link
Contributor

@FoSix FoSix commented Jan 24, 2024 •
edited by acelebanski
Loading

Description

Introduce common config variable to modules that still do not have one.

Motivation and Context

Backporting the idea of a common config var from vmseries, vmss and bootstrap modules

How Has This Been Tested?

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@FoSix FoSix linked an issue Jan 24, 2024 that may be closed by this pull request
add common map-like variable that would gather all configuration related variables to LB and VNG modules - unify zones property behaviour #5
Closed
3 tasks
@FoSix FoSix changed the base branch from main to refactor-modules January 24, 2024 14:07
@FoSix
Copy link
Contributor Author

FoSix commented Jan 24, 2024 •
edited by github-actions bot
Loading

/idempotence paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale" tf_version="1.5 1.6"

Testing job ID: 7641423330
Job result: FAILURE
Job result: SUCCESS

@FoSix FoSix force-pushed the var_grouping_for_lbs branch from 9567053 to 76fbae9 Compare January 24, 2024 15:14
@FoSix
Copy link
Contributor Author

FoSix commented Jan 24, 2024 •
edited by github-actions bot
Loading

/apply paths="examples/dedicated_vmseries_and_autoscale"

Testing job ID: 7642821397
Job result: FAILURE
Job result: SUCCESS

@acelebanski acelebanski added the refactor Related to code refactoring label Jan 25, 2024
@FoSix
Copy link
Contributor Author

FoSix commented Jan 29, 2024 •
edited by github-actions bot
Loading

/idempotence paths="examples/common_vmseries examples/common_vmseries_and_autoscale examples/dedicated_vmseries examples/dedicated_vmseries_and_autoscale examples/standalone_panorama examples/standalone_vmseries"

Testing job ID: 7695912875
Job result: FAILURE
Job result: FAILURE
Job result: SUCCESS

@FoSix FoSix force-pushed the var_grouping_for_lbs branch 2 times, most recently from acab681 to 044a90d Compare January 29, 2024 12:30
@FoSix FoSix force-pushed the var_grouping_for_lbs branch from 044a90d to 87acdbc Compare January 29, 2024 13:12
@FoSix FoSix marked this pull request as ready for review January 30, 2024 10:13
@FoSix FoSix force-pushed the var_grouping_for_lbs branch from e5c15a9 to 020042b Compare January 30, 2024 10:15
@FoSix FoSix requested a review from sebastianczech January 30, 2024 11:48
acelebanski
acelebanski reviewed Jan 30, 2024
View reviewed changes
modules/appgw/variables.tf Outdated Show resolved Hide resolved
modules/loadbalancer/variables.tf Outdated Show resolved Hide resolved
modules/virtual_network_gateway/variables.tf Outdated Show resolved Hide resolved
alperenkose
alperenkose approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@alperenkose alperenkose left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

sebastianczech
sebastianczech approved these changes Feb 29, 2024
View reviewed changes
Copy link
Contributor

@sebastianczech sebastianczech left a comment •
edited
Loading

Choose a reason for hiding this comment

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

great work - as always:)
I have small comments, but they are optional
Thank you

examples/appgw/README.md Show resolved Hide resolved
examples/appgw/README.md Show resolved Hide resolved
examples/common_vmseries/main.tf Show resolved Hide resolved
modules/appgw/outputs.tf Show resolved Hide resolved
This was linked to issues Feb 29, 2024
add EOF notation to all error_message strings in validation and precondition blocks #17
Closed
variables.tf - in validation blocks add comments describing which property is validated - for variables of object/map type, see this example #18
Closed
@acelebanski acelebanski merged commit 36ddea5 into refactor-modules Mar 1, 2024
@acelebanski acelebanski deleted the var_grouping_for_lbs branch March 1, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acelebanski acelebanski acelebanski left review comments

@sebastianczech sebastianczech sebastianczech approved these changes

@alperenkose alperenkose alperenkose approved these changes

Assignees
No one assigned
Labels
refactor Related to code refactoring
Projects
None yet
Milestone
No milestone
4 participants
@FoSix @alperenkose @sebastianczech @acelebanski