-
Notifications
You must be signed in to change notification settings - Fork 190
[do not merge] check multi value applies-to and lists #4431
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
base: main
Are you sure you want to change the base?
[do not merge] check multi value applies-to and lists #4431
Conversation
Vale Linting ResultsSummary: 5 suggestions found 💡 Suggestions (5)
|
|
@florent-leborgne I'll ask the same question as I did on #4429, what kind of reviews do you want on this? It's not the typical PR so I'm just thinking about how best to go about it 🕵️ |
|
|
||
| - **Standard case**: Our docs are aligned with the latest patch of any given minor version. That means that in most cases, we don't need to call out the exact patch version that introduced a change (that's for the release notes). | ||
|
|
||
| - **Exceptions**: In rare cases, it can happen that the change is important enough to be explicitly called out in the docs with a precise patch-level information. In that case, use two `applies_to` badges with the `!` extra symbol so that users can see clearly the versions in which the functionality is introduced. Order the `applies_to` badges starting with the latest version, and ending with the earliest version. (Automatic ordering for multiple badges is not currently supported.) |
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.
I think we talked about not explicitly documenting this / documenting it as something that should be handled in prose - it doesn't play nice with ranges so I don't know that we want to expose it
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.
I'm totally open to removing this, cc @elastic/docs-tech-leads
| * [manage and update field mappings](/solutions/observability/streams/management/schema.md) | ||
| * [identify failed and degraded documents](/solutions/observability/streams/management/data-quality.md) |
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.
any reason why you chose to make this change? I'm not sure what the current state of the feature is but it feels like we are losing info
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.
It contradicted the tagging of target pages and seemed like incorrect usage of unavailable - I asked Mike to double check
| : ([time value](elasticsearch://reference/elasticsearch/rest-apis/api-conventions.md#time-units)) Sets the time to wait before trying again if an attempt to read a [linearizable register](#repository-s3-linearizable-registers) fails. Defaults to `5s`. | ||
|
|
||
| `unsafely_incompatible_with_s3_conditional_writes` {applies_to}`stack: ga 9.2.3` | ||
| `unsafely_incompatible_with_s3_conditional_writes` {applies_to}`stack: ga 9.2.3+!` |
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.
probably practicing what we preach here, I'd just skip the !
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.
Because it's isolated and isn't a multi-patch scenario we can afford to use it though I have no strong feeling about removing it. Lmk your preference and i'll adjust
shainaraskas
left a comment
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.
approving to unblock for my teams
Co-authored-by: Brandon Morelli <[email protected]>
Co-authored-by: Brandon Morelli <[email protected]>
Summary
This PR:
.0can stay as is, or make necessary adjustments if not.Note: the
+notation is a best practice but isn't mandatory, so this PR does not update every single occurrence of applies_to that we have, and instead focuses on cases with ambiguity or that could have led to issues with the updatesThis PR should only be merged after elastic/docs-builder#2322 is merged.
CI should fail until then.
Contributes to: #4361
Generative AI disclosure