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.

Sign up for GitHub

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

Jump to bottom

InfluxDB OSS v1.11.7 #5648

Merged
merged 21 commits into from
Oct 28, 2024
Merged

InfluxDB OSS v1.11.7 #5648

merged 21 commits into from
Oct 28, 2024

Conversation

sanderson
Copy link
Collaborator

  • InfluxDB OSS v1.11.7 release notes and accompanying documentation.
  • Adding missing updates to InfluxDB Enterprise documentation.
  • Rebased/mergeable

jdstrand
jdstrand requested changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

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

I focused primarily on the changelog to make sure we mentioned the security fixes. I have a few comments inline.

content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
jdstrand
jdstrand approved these changes Oct 16, 2024
View reviewed changes
Copy link
Contributor

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

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

Approving the parts I reviewed (changelog for security).

davidby-influx
davidby-influx approved these changes Oct 16, 2024
View reviewed changes
Copy link

@davidby-influx davidby-influx left a comment

Choose a reason for hiding this comment

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

LGTM

jstirnaman
jstirnaman requested changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@jstirnaman jstirnaman 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 doing all this! I have several small suggestions and fixes.

content/enterprise_influxdb/v1/tools/influx_inspect.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/about_the_project/release-notes.md Outdated Show resolved Hide resolved
content/influxdb/v1/flux/guides/optimize-queries.md Outdated Show resolved Hide resolved
content/influxdb/v1/guides/write_data.md Outdated Show resolved Hide resolved
content/influxdb/v1/tools/api.md Outdated Show resolved Hide resolved
content/influxdb/v1/tools/api.md Outdated Show resolved Hide resolved
content/enterprise_influxdb/v1/tools/api.md Outdated Show resolved Hide resolved
@sanderson sanderson requested a review from jstirnaman October 18, 2024 22:40
jstirnaman
jstirnaman approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@jstirnaman jstirnaman left a comment

Choose a reason for hiding this comment

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

A question/suggestion about the command options formatting, but otherwise LGTM

content/enterprise_influxdb/v1/tools/influx_inspect.md
@@ -139,6 +142,31 @@ $ influx_inspect buildtsi -database mydb -datadir ~/.influxdb/data -waldir ~/.in
$ influx_inspect buildtsi -database stress -shard 1 -datadir ~/.influxdb/data -waldir ~/.influxdb/wal
```

### `check-schema`
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Shouldn't all options be optional arguments?
  • I find the brackets in the headings confusing--I had to scroll up to the previous section to find "Optional arguments are in brackets."--if we need the brackets, then I think we need the explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this was a convention established long ago in the 1.x CLI docs that I've always hated. At one point, I updated the InfluxDB Enterprise CLI docs to correct for this. I just never went back and updated this doc and didn't want to hang this PR up on correcting that convention. But I agree. It's not good.

While we're talking about it, there are two conventions that have always bugged me in the 1.x docs:

Because they're headings and many of these are referenced in links, these changes have cascading affects that I didn't feel I had time to include in this PR.

@sanderson sanderson merged commit 91482e6 into master Oct 28, 2024
1 check passed
@sanderson sanderson deleted the influxdb-oss-1.11.7 branch October 28, 2024 21:13
@beheh beheh mentioned this pull request Oct 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidby-influx davidby-influx davidby-influx approved these changes

@jdstrand jdstrand jdstrand approved these changes

@jstirnaman jstirnaman jstirnaman approved these changes

@gwossum gwossum Awaiting requested review from gwossum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sanderson @jstirnaman @jdstrand @davidby-influx