-
Notifications
You must be signed in to change notification settings - Fork 64
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
Draft Config Refactor #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these changes! They clean things up significantly.
We should split this up to two separate PRs. One for the config refactor and one for the private cluster support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like these changes, tested them locally and everything looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
just some test issues with unsubstituted variable for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
At the core of this PR are the changes made to the DraftConfig{} struct. The BuilderVarDefault{} struct was absorbed into BuilderVar{}. This led to a restructuring of all draft.yaml templates as well as a refactoring of several functions to incorporate the new data structure.
Fixes # (issue)
Feature # (details)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. Is it a breaking change which will impact consuming tool(s).
Checklist: