Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

feat(example/vmseries_combined): Refactored example for vmseries_combined #305

Merged
merged 22 commits into from
May 23, 2023

Conversation

mariuszgebala
Copy link
Contributor

Description

Improve the tgw_inbound_combined_with_gwlb example

Motivation and Context

The need to change the name and add a more detailed description of the example

Types of changes

  • New feature

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

@mariuszgebala mariuszgebala requested a review from a team as a code owner April 19, 2023 09:36
Copy link
Contributor

@pimielowski pimielowski left a comment

Choose a reason for hiding this comment

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

Looks good :)

@migara
Copy link
Member

migara commented Apr 26, 2023

Closes #298 combined_vmseries

Copy link
Contributor

@sebastianczech sebastianczech left a comment

Choose a reason for hiding this comment

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

LGTM , but I have a question @mariuszgebala - I know , that in the scope of #298 it was renaming the example and improving README, but what do you think about changing approach in the code to make resource dynamically, based on values in tfvars ? We don't need to do it in this PR - maybe let's create new issue and improve that example in new PR ?

Copy link
Member

Choose a reason for hiding this comment

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

@mariuszgebala can you please change the LB to ALB as we discussed before we merge this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@mariuszgebala
Copy link
Contributor Author

LGTM , but I have a question @mariuszgebala - I know , that in the scope of #298 it was renaming the example and improving README, but what do you think about changing approach in the code to make resource dynamically, based on values in tfvars ? We don't need to do it in this PR - maybe let's create new issue and improve that example in new PR ?

Done

Copy link
Contributor

@sebastianczech sebastianczech left a comment

Choose a reason for hiding this comment

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

Could we change name of the PR in order to follow conventional commits ? Besides that it looks ok 👍

Copy link
Contributor

@lstadnik lstadnik left a comment

Choose a reason for hiding this comment

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

LGTM

@mariuszgebala mariuszgebala changed the title Improve tgw_inbound_combined_with_gwlb example feat(example/vmseries_combined): Refactored example for vmseries_combined May 8, 2023
@migara migara merged commit c1f9db0 into main May 23, 2023
@migara migara deleted the update-gwlb-tgw-combined branch May 23, 2023 12:34
@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.4.2 🎉

The release is available on Terraform Registry and GitHub release

Posted by semantic-release bot

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants