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

feat: add *WithFilter helper functions #1353

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

Conversation

AgustinRamiroDiaz
Copy link

@AgustinRamiroDiaz AgustinRamiroDiaz commented Oct 5, 2023

Description

Fixes #1021

I've tested this in my local test suite and it works 👌

Question for the reviewers: would it make sense to approach this with the builder design pattern? It'd break the currently API but I think it'd be easier to maintain to add more of this configurations/features and more configurable

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added Copy*WithFilter family funcitons like CopyTerraformFolderToTempWithFilter

Signed-off-by: Agustín Díaz <agustin.ramiro.diaz@gmail.com>
Signed-off-by: Agustín Díaz <agustin.ramiro.diaz@gmail.com>
Signed-off-by: Agustín Díaz <agustin.ramiro.diaz@gmail.com>
Signed-off-by: Agustín Díaz <agustin.ramiro.diaz@gmail.com>
@AgustinRamiroDiaz AgustinRamiroDiaz changed the title [WIP] feat: add *WithFilter helper functions feat: add *WithFilter helper functions Oct 5, 2023
@denis256
Copy link
Member

Hello,
to reduce the number of function arguments, can be introduced options struct which will have inside all required fields, to keep backward compatibility - existing functions can instantiate options struct and pass it to functions with options argument.

It would also be useful to have tests that can check if the new functions will keep working in the future.

Reference:
https://github.com/gruntwork-io/terratest/blob/master/modules/helm/options.go

@AgustinRamiroDiaz
Copy link
Author

to reduce the number of function arguments, can be introduced options struct which will have inside all required fields, to keep backward compatibility - existing functions can instantiate options struct and pass it to functions with options argument.

Great idea! Do you think it'd be useful to do that?

It would also be useful to have tests that can check if the new functions will keep working in the future.

I'll try to add some tests 👍

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.

Copy hidden directories using CopyTerraformFolderToTemp
2 participants