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

Documentation and CHANGELOG.md entry for #2579. #2618

Merged
merged 4 commits into from
May 18, 2020
Merged

Documentation and CHANGELOG.md entry for #2579. #2618

merged 4 commits into from
May 18, 2020

Conversation

pstibrany
Copy link
Contributor

This PR adds CHANGELOG entry and documentation for PR #2579.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Perfect! Just DCO & LGTM!

@pstibrany
Copy link
Contributor Author

Perfect! Just DCO & LGTM!

🤦

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

@pstibrany One thing I have realized, we mentioned and documented config file to use it, however the flag itself is not documented and it is hidden (if it hasn't changed). This might create confusion. Should we also make it visible or at least mention it somewhere?

@pstibrany
Copy link
Contributor Author

@pstibrany One thing I have realized, we mentioned and documented config file to use it, however the flag itself is not documented and it is hidden (if it hasn't changed). This might create confusion. Should we also make it visible or at least mention it somewhere?

That's a good point. I will put it into documentation and changelog entry.

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

🥇

```

`backend_config` field for memcached supports all the same configuration as memcached for [index cache](#memcached-index-cache).

`caching_config` is a configuration for chunks cache and supports the following optional settings:
`caching_config` is a configuration for cache and supports the following optional settings for chunks caching:
Copy link
Member

@GiedriusS GiedriusS May 17, 2020

Choose a reason for hiding this comment

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

But caching_config key doesn't exist, right? 🤔 It has been moved a level up. So, I think this line should be reworded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct. I've removed mention of caching_config.

Signed-off-by: Peter Štibraný <[email protected]>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM (just left a nit)

docs/components/store.md Outdated Show resolved Hide resolved
Signed-off-by: Peter Štibraný <[email protected]>
@pracucci pracucci merged commit 6cfcb34 into thanos-io:master May 18, 2020
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.

6 participants