-
Notifications
You must be signed in to change notification settings - Fork 33
Applies_to: Expand versioning features #2322
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?
Conversation
…r applicabilities
🔍 Preview links for changed docs |
…out a specified version
| ProductLifecycle.Removed => | ||
| $"We plan to remove this functionality in a future {applicabilityDefinition.DisplayName} update. Subject to change.", | ||
| _ => tooltipText | ||
| }; | ||
| } | ||
| else | ||
| { | ||
| // No version specified - check if we should show base version | ||
| tooltipText = versioningSystem.Base.Major != AllVersionsSpec.Instance.Min.Major | ||
| ? applicability.Lifecycle switch | ||
| { | ||
| ProductLifecycle.Removed => | ||
| $"Removed in {applicabilityDefinition.DisplayName} {versioningSystem.Base.Major}.{versioningSystem.Base.Minor}.", | ||
| _ => | ||
| $"{lifecycleFull} since {versioningSystem.Base.Major}.{versioningSystem.Base.Minor}." | ||
| } | ||
| : $"{lifecycleFull} on {applicabilityDefinition.DisplayName} unless otherwise specified."; | ||
| : $"{lifecycleFull} on {applicabilityDefinition.DisplayName} unless otherwise specified."; | ||
| } |
reakaleek
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.
There are two test cases that made me curious.
Otherwise, LGTM.
Nice work.
| "sub_type": "stack", | ||
| "lifecycle": "ga", | ||
| "version": "9999.9999.9999" | ||
| "version": "all" |
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.
➕ thanks for fixing this and making it properly.
|
I'm wondering a bit why there are so many warnings. What's the reason we can't do this backwards compatible? |
|
@reakaleek The main factor is the decision to change the default behavior of a version assignment. In short: Example: https://github.com/elastic/elasticsearch/blob/main/docs/reference/search-connectors/elastic-managed-connectors.md?plain=1 triggers two warnings. One regarding multiple applications using greater-than-or-equal syntax, and one about range overlap.
|
| markdown |> convertsToHtml """ | ||
| <p class="applies applies-block"> | ||
| <span class="applicable-info" data-tippy-content="Not available on Elastic Stack unless otherwise specified."> | ||
| <span class="applicable-info" data-tippy-content="Unavailable since 8.0."> |
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.
Unavailable should never show a version - we may have missed this case in the specs; now that I think of it
If something is no longer in the product, we'd use "removed 8.0", with the removed lifecycle.
Unavailable strictly means that this is simply not possible to do with that product
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.
Ah, that portion of the testing suite is still pending an update. I'll make sure the version doesn't show up in the final design.
…rsion mighyt cause confusion
This PR provides a set of adjustments to the inner workings of versioning in the context of applies_to. This draft will be further updated with front-end changes, but badge-related changes can be checked independently.
It is part of #2135.
Problem description
Describing versions within applicability contains several quirks and limitations, such as:
Proposed changes
A new
VersionSpecclass has been created. It builds on SemVersion, and supports the following versioning types:9.5.0, 9.5.1, 9.5.6 -> all display as 9.5 or 9.5+
When no version is specified for versioned products, the base version is now shown:
Before:
stack: ga-> Badge shows just "Stack"After:
stack: ga-> Badge shows "Stack│8.0+" (assuming base = 8.0)Unversioned products continue to show no version.
Implemented validation in
ApplicableToYamlConverter: