8000 Delete unused test Cfn templates by viren-nadkarni · Pull Request #11771 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Delete unused test Cfn templates #11771

8000
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
Merged
merged 3 commits into from
Nov 6, 2024
Merged

Delete unused test Cfn templates #11771

merged 3 commits into from
Nov 6, 2024

Conversation

viren-nadkarni
Copy link
Member
@viren-nadkarni viren-nadkarni commented Nov 1, 2024 •
edited
Loading

This PR removes several unused Cfn test templates.

8000
@viren-nadkarni viren-nadkarni self-assigned this Nov 1, 2024
@viren-nadkarni viren-nadkarni added the semver: patch Non-breaking changes which can be included in patch releases label Nov 1, 2024
@viren-nadkarni viren-nadkarni marked this pull request as ready for review November 1, 2024 10:36
@github-actions GitHub Actions
Copy link
github-actions bot commented Nov 1, 2024 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 31s ⏱️ +27s
3 532 tests ±0  3 118 ✅ ±0  414 💤 ±0  0 ❌ ±0 
3 534 runs  ±0  3 118 ✅ ±0  416 💤 ±0  0 ❌ ±0 

Results for commit 9adf8de. ± Comparison against base commit bcdb73b.

♻️ This comment has been updated with latest results.

bentsku
bentsku reviewed Nov 5, 2024
View reviewed changes
Copy link
Contributor
@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

Thanks for updating it! I've tried to look for the usage of the template28.yaml but couldn't find any. Is this template still used?

@viren-nadkarni
Copy link
Member Author

Thanks for updating it! I've tried to look for the usage of the template28.yaml but couldn't find any. Is this template still used?

@bentsku You're right, it seems like most of these template**.yaml files are no longer used. I'll delete them and see if anything breaks

@viren-nadkarni viren-nadkarni marked this pull request as draft November 5, 2024 06:35
@viren-nadkarni viren-nadkarni changed the title Update EC2 AMI in Cfn templates Delete unused test Cfn templates Nov 5, 2024
@viren-nadkarni viren-nadkarni marked this pull request as ready for review November 5, 2024 11:53
@viren-nadkarni
8000 Copy link
Member Author

@bentsku Thanks for highlighting this. This PR now removes all the unused Cfn templates.

bentsku
bentsku approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor
@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

🧹! LGTM, I think this is a nice cleanup, hard to keep track of those kind of dependencies when they all live in their own folder, and also pretty hard to know where it is used.

None of them were used in upstream dependencies, right? It does not look like it 👌

Maybe Dominik wants to chime in too just in case, but I think we lost the context of those files and they are not worth a lot anymore.

dominikschubert
dominikschubert approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member
@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM, tried to find references for a few of the removed files but also couldn't find any, so seems good! Thanks for cleaning that up 🧹

@viren-nadkarni viren-nadkarni merged commit 359f45f into master Nov 6, 2024
40 checks passed
@viren-nadkarni viren-nadkarni deleted the ec2-ami-eol branch November 6, 2024 12:34
zaingz pushed a commit that referenced this pull request Nov 11, 2024
@viren-nadkarni @zaingz
Delete unused test Cfn templates (#11771)
0de36aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dominikschubert dominikschubert dominikschubert approved these changes

@bentsku bentsku bentsku approved these changes

Assignees

@viren-nadkarni viren-nadkarni

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@viren-nadkarni @dominikschubert @bentsku
0