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

chore(workgroup's bucket): Make it more flexible for imports #32

Merged
merged 2 commits into from
Aug 25, 2023
Merged

chore(workgroup's bucket): Make it more flexible for imports #32

merged 2 commits into from
Aug 25, 2023

Conversation

egarbi
Copy link
Contributor

@egarbi egarbi commented Jul 26, 2023 •
edited
Loading

All these changes have been suggested by @jtsaito here

  • Remove Glue component. The table will be managed from outside the module.
  • Drop workspace_bucket_prefix and rely in workgroup_bucket from now on
  • Modify test case to support the new input name

@egarbi egarbi force-pushed the make-module-more-flexible branch from 54ba05a to c159d59 Compare July 26, 2023 19:04
@egarbi egarbi changed the title chore(workgroup's bucket): Make it more flexible for import chore(workgroup's bucket): Make it more flexible for imports Jul 26, 2023
@egarbi
chore(workgroup's bucket): Make it more flexible for import
428186d 
All these changes have been suggested by @jtsaito in babbel#28
- Remove Glue component. The table will be managed from outside the module.
- Drop workspace_bucket_prefix and rely in workgroup_bucket from now on
@egarbi egarbi force-pushed the make-module-more-flexible branch from 8a4465a to 428186d Compare July 26, 2023 19:10
@jansiwy jansiwy requested a review from jtsaito July 26, 2023 19:16
@egarbi egarbi mentioned this pull request Jul 26, 2023
Closed
@jtsaito jtsaito self-assigned this Aug 24, 2023
jtsaito
jtsaito approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@jtsaito jtsaito left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@egarbi Sorry for the delay and thank you again for your contributing!

The changes are fine. I'll update the change log and create a release after merging.

Cf. #32 (comment)

@jtsaito
Copy link
Contributor

jtsaito commented Aug 24, 2023

@egarbi Sorry, the tests are still failing. Could you please addjust https://github.com/babbel/terraform-aws-athena/blob/main/_test/main.tf according to your changes.

@egarbi
Copy link
Contributor Author

egarbi commented Aug 24, 2023

@egarbi Sorry, the tests are still failing. Could you please addjust https://github.com/babbel/terraform-aws-athena/blob/main/_test/main.tf according to your changes.

done

@jtsaito jtsaito merged commit 771e190 into babbel:main Aug 25, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtsaito jtsaito jtsaito approved these changes

Assignees

@jtsaito jtsaito

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@egarbi @jtsaito