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

Deprecate configtest and add validate command #2732

Merged
merged 3 commits into from
Feb 11, 2017
Merged

Conversation

kyhavlov
Copy link
Contributor

Hides the configtest command, adds a deprecation notice, and adds the validate command with the old configtest flags as hidden flags.

Closes #2421

@slackpad
Copy link
Contributor

LGTM with one addition - missed the success message and -quiet part of #2421.

@slackpad
Copy link
Contributor

LGTM

@kyhavlov kyhavlov merged commit 466e39d into master Feb 11, 2017
@kyhavlov kyhavlov deleted the f-validate-command branch February 11, 2017 01:34
var quiet bool

f := c.Command.NewFlagSet(c)
f.Var((*agent.AppendSliceValue)(&configFiles), "config-file",
Copy link
Contributor

Choose a reason for hiding this comment

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

@kyhavlov I still think it'd be a better UX to just accept one option here and determine if it's a file or folder appropriately. I don't see a reason why we need to treat them separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left those in as hidden flags to stay compatible with configtest; everything (args, -config-file, config-dir) gets added to configFiles which works with files or directories. Do you think we should just remove the flags from validate altogether?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry. I didn't see the "hidden" call below. +1 to BC! Great work on this and sorry for the confusion

@mterron
Copy link
Contributor

mterron commented Feb 25, 2018

consul validate seem to fail is there's no 'data_dir' in the config file even though you could just use a cli option to provide that parameter.

/ # consul validate /etc/consul/
Config validation failed: data_dir cannot be empty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants