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

Follow-up on #305 #310

Merged
merged 2 commits into from Nov 9, 2018

Conversation

@errordeveloper
Copy link
Member

commented Nov 8, 2018

Description

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All tests passing (i.e. make test)

@errordeveloper errordeveloper force-pushed the subnets branch 2 times, most recently from 896e3c0 to 6687ae3 Nov 8, 2018

@errordeveloper errordeveloper requested review from gruebel and richardcase and removed request for gruebel Nov 8, 2018

@errordeveloper errordeveloper force-pushed the subnets branch 2 times, most recently from 1d1097a to f6d7b46 Nov 8, 2018

pkg/vpc/vpc.go Outdated Show resolved Hide resolved
Create `vpc.ImportSubnets`
- refactor all VPC helpers into new package
- make sure to only use the provider interface
Fix subnet ordering assumption
- pass full provider to stack manager and cluster stack builder
@stefanprodan
Copy link
Member

left a comment

LGTM

Nice refactoring @errordeveloper having a vpc package makes the code easier to understand

@richardcase
Copy link
Collaborator

left a comment

Looks good to me!

@errordeveloper errordeveloper merged commit 39b2ca6 into master Nov 9, 2018

3 checks passed

GolangCI No issues found!
Details
WIP Ready for review
Details
ci/circleci: make-eksctl-image Your tests passed on CircleCI!
Details
@errordeveloper

This comment has been minimized.

Copy link
Member Author

commented Nov 9, 2018

@richardcase @stefanprodan thanks for the review, I'm looking forward to this weeks release with all VPC customisations some users been waiting for!

@errordeveloper errordeveloper deleted the subnets branch Nov 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.