Skip to content

Add m7g and r7g as ARM instance types#6436

Merged
TiberiuGC merged 2 commits intoeksctl-io:mainfrom
TiberiuGC:bugfix/amend-arm-instance-types
Mar 17, 2023
Merged

Add m7g and r7g as ARM instance types#6436
TiberiuGC merged 2 commits intoeksctl-io:mainfrom
TiberiuGC:bugfix/amend-arm-instance-types

Conversation

@TiberiuGC
Copy link
Copy Markdown
Contributor

@TiberiuGC TiberiuGC commented Mar 15, 2023 •
edited
Loading

Description

Closes #6377

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@Himangini
Copy link
Copy Markdown
Contributor

Himangini commented Mar 16, 2023

@TiberiuGC this is still in draft 😅
do we need to add tests for these new instance types?

@TiberiuGC
Copy link
Copy Markdown
Contributor Author

TiberiuGC commented Mar 16, 2023 •
edited
Loading

@TiberiuGC this is still in draft 😅 do we need to add tests for these new instance types?

Yeah I forgot that if you open a draft PR once it defaults to draft for subsequent PRs 😓
Hmm, I don't see any of the IsXXXXInstanceType() functions having any tests. They are quite basic tho 🤔 Do you have any suggestions for where tests would be worth it?

@TiberiuGC TiberiuGC marked this pull request as ready for review March 16, 2023 13:41
@Himangini
Copy link
Copy Markdown
Contributor

Himangini commented Mar 16, 2023

@TiberiuGC this is still in draft 😅 do we need to add tests for these new instance types?

Yeah I forgot that if you open a draft PR once it defaults to draft for subsequent PRs 😓 Hmm, I don't see any of the IsXXXXInstanceType() functions having any tests. They are quite basic tho 🤔 Do you have any suggestions for where tests would be worth it?

Nah, it's fine. 👍🏻

@TiberiuGC TiberiuGC enabled auto-merge (squash) March 17, 2023 06:41
@TiberiuGC TiberiuGC merged commit a6767ed into eksctl-io:main Mar 17, 2023
@yuxiang-zhang yuxiang-zhang mentioned this pull request Jan 24, 2024
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cPu1 cPu1 cPu1 approved these changes

@Himangini Himangini Himangini approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

kind/bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] m7g and r7g are not handled by IsARMInstanceType

3 participants

@TiberiuGC @Himangini @cPu1