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

Refactor: Rename variables to inputs [CFG-1500] #19

Merged
merged 2 commits into from
Mar 3, 2022

Conversation

ofekatr
Copy link
Contributor

@ofekatr ofekatr commented Feb 28, 2022

What this does

  • Refactors types for storing input values, local values, and TF variables in the terraform package.
    This change helps clarify the contents of different variable-related structs and maps in our current parser's flow.
  • Renames several functions and types with the variable term, to use the term input instead.
    This change helps gain better distinguishment between inputs and locals in the code, as both are considered variables in Terraform.

More information

@ofekatr ofekatr changed the title Refactor: Refactored variables term to inputs [CFG-1500] Refactor: Rename variables to inputs [CFG-1500] Feb 28, 2022
@ofekatr ofekatr requested a review from ipapast February 28, 2022 12:03
@ofekatr ofekatr marked this pull request as ready for review February 28, 2022 12:03
@ofekatr ofekatr requested a review from a team as a code owner February 28, 2022 12:03
@ofekatr ofekatr force-pushed the refactor/rename-variables-to-inputs branch 3 times, most recently from d25805b to c2506de Compare March 1, 2022 11:32
@ofekatr ofekatr force-pushed the refactor/rename-variables-to-inputs branch from c2506de to c4fe055 Compare March 1, 2022 11:37
Copy link
Contributor

@ipapast ipapast left a comment

Choose a reason for hiding this comment

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

I think the general refactor is good, I'm only concerned about the word inputs as we are completely removing the variables word here. What do you think of inputVariables instead? So someone that looks at this the first time understands what it is and it still aligned with the Terraform vocabulary.

I think It's the middle ground. What do you think? @ofekatr @YairZ101

@YairZ101 YairZ101 force-pushed the refactor/rename-variables-to-inputs branch from 0dfd8cd to 4504ef1 Compare March 3, 2022 09:09
@YairZ101
Copy link
Contributor

YairZ101 commented Mar 3, 2022

I think the general refactor is good, I'm only concerned about the word inputs as we are completely removing the variables word here. What do you think of inputVariables instead? So someone that looks at this the first time understands what it is and it still aligned with the Terraform vocabulary.

I think It's the middle ground. What do you think? @ofekatr @YairZ101

I've renamed all the places we used inputs and changed them to inputVariables as you suggested.
Can you please give it a 2nd look to check I didn't miss anything?

@ipapast
Copy link
Contributor

ipapast commented Mar 3, 2022

I think the general refactor is good, I'm only concerned about the word inputs as we are completely removing the variables word here. What do you think of inputVariables instead? So someone that looks at this the first time understands what it is and it still aligned with the Terraform vocabulary.
I think It's the middle ground. What do you think? @ofekatr @YairZ101

I've renamed all the places we used inputs and changed them to inputVariables as you suggested. Can you please give it a 2nd look to check I didn't miss anything?

it looks okay to me, I will trust your editor for renaming more than my eyes :) thank you!

Copy link
Contributor

@ipapast ipapast left a comment

Choose a reason for hiding this comment

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

🚢

@YairZ101 YairZ101 merged commit 93d5d83 into main Mar 3, 2022
@YairZ101 YairZ101 deleted the refactor/rename-variables-to-inputs branch March 3, 2022 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants