-
Notifications
You must be signed in to change notification settings - Fork 536
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
Adding CLI for the migration tool #154
Conversation
/cc @chrisohaver @fturib |
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.
Thanks for moving to function instead of global vars.
Please see my other comments.
} | ||
|
||
// defaultCorefileFromPath takes the path where the Corefile is located and checks | ||
// if the Corefile is the default for a that version of Kubernetes. |
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.
typo with "for a"
// defaultCorefileFromPath takes the path where the Corefile is located and checks | ||
// if the Corefile is the default for a that version of Kubernetes. | ||
func defaultCorefileFromPath(k8sVersion, corefilePath string) (bool, error) { | ||
if _, err := os.Stat(corefilePath); os.IsNotExist(err) { |
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.
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.
Done!
Short: "A brief description of your application", | ||
Long: dedent.Dedent(` | ||
|
||
┌──────────────────────────────────────────────────────────┐ |
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.
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.
Readme I think is missing some information.
|
||
where `command`, `flags` are: | ||
|
||
- `command`: The operation you want to perform. |
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.
Need to define what the valid commands are. I think these are in the "Operations" section, but the terminology is not consistent command/operation, so it's confusing.
where `command`, `flags` are: | ||
|
||
- `command`: The operation you want to perform. | ||
- `flags` : Specifies flags required to carry out the operations. |
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.
Need to define what the valid flags are per command.
kubernetes/corefile-tool/README.md
Outdated
``` | ||
|
||
```bash | ||
# Migrate CoreDNS from v1.4.0 to v1.5.0 and handle deprecations . |
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.
Should explain what "handle deprecations" means.
Can you also add a wrapper for Released()? |
Adds the CLI as defined here
Adds support to basic commands such as
migrate
,deprecated
,default
,removed
,unsupported
andvaildversions