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

fix: various improvements to setup command #20155

Merged
merged 2 commits into from
Dec 2, 2020
Merged

Conversation

adrian-thurston
Copy link
Contributor

When checking for existing configs, check the length of the array after
parsing. There could be an empty config file present.

If --name was specified then use that as the configuration name, regardless of
whether or not there are existing configs.

Allow a 0 (infinite) retention to be specified with --retention when in
interactive mode.

@danxmoran danxmoran self-requested a review November 23, 2020 21:22
Copy link
Contributor

@danxmoran danxmoran left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes, they look great! Could you add a line to CHANGELOG.md?

cmd/influx/setup.go Outdated Show resolved Hide resolved
cmd/influx/setup.go Outdated Show resolved Hide resolved
cmd/influx/setup.go Outdated Show resolved Hide resolved
@danxmoran danxmoran self-assigned this Dec 2, 2020
@danxmoran
Copy link
Contributor

I'll get this one over the finish line.

When checking for existing configs, check the length of the array after
parsing. There could be an empty config file present.

If --name was specified then use that as the configuration name, regardless of
whether or not there are existing configs.

Allow a 0 (infinite) retention to be specified with --retention when in
interactive mode.
@danxmoran danxmoran force-pushed the fix/setup-command branch 2 times, most recently from d7f8d98 to 8465857 Compare December 2, 2020 15:00
Copy link
Contributor

@ayang64 ayang64 left a comment

Choose a reason for hiding this comment

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

Looks great!

@danxmoran danxmoran merged commit e0543e6 into master Dec 2, 2020
@danxmoran danxmoran deleted the fix/setup-command branch December 2, 2020 15:40
danxmoran added a commit that referenced this pull request Dec 2, 2020
When checking for existing configs, check the length of the array after
parsing. There could be an empty config file present.

If --name was specified then use that as the configuration name, regardless of
whether or not there are existing configs.

Allow a 0 (infinite) retention to be specified with --retention when in
interactive mode.

Co-authored-by: Dan Moran <dmoran@influxdata.com>
@adrian-thurston
Copy link
Contributor Author

Thanks @danxmoran and @ayang64. Came back today to work on it and saw you took care of it. I've been wrapping up a move into a new apartment and haven't been moving on things as quickly as I'd like!

@danxmoran
Copy link
Contributor

No worries 😄 hope the move went well!

danxmoran added a commit that referenced this pull request Dec 3, 2020
When checking for existing configs, check the length of the array after
parsing. There could be an empty config file present.

If --name was specified then use that as the configuration name, regardless of
whether or not there are existing configs.

Allow a 0 (infinite) retention to be specified with --retention when in
interactive mode.

Co-authored-by: Dan Moran <dmoran@influxdata.com>
danxmoran added a commit that referenced this pull request Dec 3, 2020
When checking for existing configs, check the length of the array after
parsing. There could be an empty config file present.

If --name was specified then use that as the configuration name, regardless of
whether or not there are existing configs.

Allow a 0 (infinite) retention to be specified with --retention when in
interactive mode.

Co-authored-by: Dan Moran <dmoran@influxdata.com>
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.

3 participants