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 parameter parsing #356

Merged
merged 49 commits into from
Apr 21, 2024
Merged

Refactor parameter parsing #356

merged 49 commits into from
Apr 21, 2024

Conversation

MatrixCrawler
Copy link
Collaborator

@MatrixCrawler MatrixCrawler commented Apr 3, 2024 •
edited
Loading

This is my attempt in reorganizing the parameter parsing.
i extracted the different configuration parsers in single files for better overview and testing capabilities.
Here are some things i explicitly tested:

  • parsing version from
    • commandline
    • toml file
    • tfswitch file
    • terraform-version file
    • version.tf
    • terragrunt file
    • environment variable

What i did not do is adding any more parameters to the i,e, toml file.
But this should be more easily doable with the refactored code.

@MatrixCrawler MatrixCrawler force-pushed the refactor-params branch 2 times, most recently from 8c98f49 to 3acbb84 Compare April 3, 2024 12:58
@MatrixCrawler MatrixCrawler marked this pull request as draft April 3, 2024 13:05
@MatrixCrawler MatrixCrawler force-pushed the refactor-params branch 6 times, most recently from 18b9ecb to 5837518 Compare April 3, 2024 14:00
yermulnik
yermulnik reviewed Apr 3, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
lib/param_parsing/environment_test.go Outdated Show resolved Hide resolved
lib/param_parsing/parameters.go Outdated Show resolved Hide resolved
lib/param_parsing/parameters.go Outdated Show resolved Hide resolved
lib/param_parsing/parameters.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
test-data/test_precedence/terragrunt.hcl Outdated Show resolved Hide resolved
test-data/test_terragrunt_hcl/terragrunt.hcl Outdated Show resolved Hide resolved
This was linked to issues Apr 4, 2024
mirror keyword does nothing in .tfswitch.toml #274
Closed
Clarify the logic of display-only flags when the .tfswitch.toml config file provided #302
Closed
Parse terragrunt.hcl and inherit terraform_version_constraint from parent #259
Open
@yermulnik yermulnik mentioned this pull request Apr 4, 2024
Closed
@MatrixCrawler MatrixCrawler linked an issue Apr 4, 2024 that may be closed by this pull request
using terraform_version_constraint in terragrunt.hcl doesn't seem to cause terraform version to change #220
Closed
@MatrixCrawler MatrixCrawler removed a link to an issue Apr 4, 2024
Parse terragrunt.hcl and inherit terraform_version_constraint from parent #259
Open
@MatrixCrawler MatrixCrawler added the enhancement Refactor existing code for better performance and quality label Apr 5, 2024
@MatrixCrawler MatrixCrawler marked this pull request as ready for review April 5, 2024 13:38
@MatrixCrawler MatrixCrawler mentioned this pull request Apr 5, 2024
Closed
@MatrixCrawler MatrixCrawler self-assigned this Apr 5, 2024
yermulnik
yermulnik reviewed Apr 5, 2024
View reviewed changes
Copy link
Collaborator

@yermulnik yermulnik left a comment

Choose a reason for hiding this comment

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

JFYI: the reason I push %q for file names and paths is that sometimes they consist of special chars (e.g. spaces or linebreaks) and wrapping them into double quotes makes it obvious where the start and the end of the string is.

README.md Outdated Show resolved Hide resolved
lib/defaults.go Outdated Show resolved Hide resolved
lib/param_parsing/terraform_version.go Outdated Show resolved Hide resolved
lib/param_parsing/terraform_version.go Outdated Show resolved Hide resolved
lib/param_parsing/terragrunt.go Outdated Show resolved Hide resolved
lib/param_parsing/versiontf.go Outdated Show resolved Hide resolved
lib/param_parsing/versiontf.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
MatrixCrawler and others added 22 commits April 21, 2024 10:15
@MatrixCrawler
- fix imports and fix cancellation of prompt
ef6418e
@MatrixCrawler
- fix race condition with waiting for deferred file closes and renami…
05bd380 
…ng target file which would occasionally lead to errors due to existing file handler

- removed unnecessary os.Exit
- added debug logging
@MatrixCrawler
- update to hcl/v2 package
cf1e230
@MatrixCrawler @yermulnik
Apply suggestions from code review
09fed37 
Co-authored-by: George L. Yermulnik <yz@yz.kiev.ua>
@MatrixCrawler @yermulnik
Apply suggestions from code review
778ffcc 
Co-authored-by: George L. Yermulnik <yz@yz.kiev.ua>
@MatrixCrawler
- added log-level parameter
753cb53
@MatrixCrawler
- moved all utility functions out of main.
ef175f6 
- refactored functions to private where possible.
- removed main_test as it was not really a useful test.
@MatrixCrawler
- added log-level to toml file
dee1bd6
@MatrixCrawler
- sort if block alphabetically
3c75651
@MatrixCrawler
- added test for defect terragrunt decalaration
8286b20 
- added test for non existing file for terragrunt and .terraform-version file
@MatrixCrawler
- added test for non existing file for .tfswitchrc file
cb39011
@MatrixCrawler
- added test for non existing and erroneous file for tfswitch.toml file
bc11354
@MatrixCrawler
- added test for non existing, erroneous terraform module file
5f7b627 
- added test for non existent version in terraform
@MatrixCrawler
- fixed comment counter
7dfa3c9 
- deferred files closures
@MatrixCrawler
- added an exception for integration tests as an empty or an erroneou…
f3ba257 
…s file would lead to showing the dropdown list which cannot be handled in an integration test, thus the test runs infinitely
@MatrixCrawler
- added integration tests for windows
d039a67
@MatrixCrawler
Revert "- added integration tests for windows"
30710fc 
This reverts commit 32dc75c.
@MatrixCrawler
- use filepath.Join instead of concatenating strings
12ef46d
@MatrixCrawler
- use filepath.Join instead of concatenating strings
34e13d8
@MatrixCrawler
- use filepath.Join instead of concatenating strings
48b744b
@MatrixCrawler
- replaced if else with switch statement
7d4d411
@MatrixCrawler
- remove commented lines
a08c5ee
@MatrixCrawler MatrixCrawler merged commit 9059ac6 into warrensbox:master Apr 21, 2024
@MatrixCrawler MatrixCrawler deleted the refactor-params branch April 21, 2024 08:20
@MatrixCrawler MatrixCrawler mentioned this pull request Apr 22, 2024
Closed
@MatrixCrawler MatrixCrawler linked an issue Apr 22, 2024 that may be closed by this pull request
Add a flag to return the path to the binary #217
Closed
@MatrixCrawler MatrixCrawler mentioned this pull request Apr 23, 2024
Closed
@yermulnik
Copy link
Collaborator

@MatrixCrawler I'm trying to figure out whether it's me or tfswitch — the log-level from TOML isn't being taken into account:

> fgrep log ~/.tfswitch.toml
log-level = "DEBUG"

> tfswitch --version
19:20:58.844 INFO Reading configuration from "/home/giermulnik/.tfswitch.toml"
Version: v1.1.0

> tfswitch --version -g DEBUG
19:21:06.483 INFO [toml.go:15,getParamsTOML] Reading configuration from "/home/giermulnik/.tfswitch.toml"
Version: v1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yermulnik yermulnik yermulnik left review comments

@warrensbox warrensbox warrensbox left review comments

@crablab crablab crablab approved these changes

@jukie jukie Awaiting requested review from jukie jukie is a code owner

@MatthewJohn MatthewJohn Awaiting requested review from MatthewJohn MatthewJohn is a code owner

Assignees

@MatrixCrawler MatrixCrawler

Labels
enhancement Refactor existing code for better performance and quality
Projects
None yet
Milestone
Release 1.1.0
5 participants
@MatrixCrawler @yermulnik @warrensbox @crablab @MatthewJohn