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

fix: Updated the object-based configuration in variable.tf #79

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Bhomesh
Copy link

@Bhomesh Bhomesh commented Nov 18, 2024

PLEASE DO NOT MERGE THIS PR

what

  • Converted EC2 instance configuration from individual variables to a structured object type
  • Introduced new instance_configuration object to consolidate all EC2-related settings
  • Created spot_configuration object to group spot instance specific parameters
  • Updated resource configurations to use the new object structure
  • Added default values and optional fields to maintain backward compatibility

why

  • Improves code maintainability by grouping related configurations together
  • Reduces the number of individual variables that need to be managed
  • Makes it easier to validate input configurations with proper type constraints
  • Simplifies module usage by providing a more structured and intuitive interface
  • Follows HashiCorp's recommended practices for variable organization
  • Makes it easier to extend the module with new instance configurations in the future

Copy link
Member

@nileshgadgi nileshgadgi left a comment

Choose a reason for hiding this comment

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

LGTM!

@h1manshu98 h1manshu98 self-requested a review November 18, 2024 22:00
Copy link
Member

@h1manshu98 h1manshu98 left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

3 participants