-
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
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? |
…rsion mighyt cause confusion
| [ProductLifecycle.Development] = new("Development", "Development", 7), | ||
| [ProductLifecycle.Discontinued] = new("Discontinued", "Discontinued", 8), | ||
| }; | ||
| } |
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.
Sorry if I'm commenting on something that has already been decided.
I think ordering by product lifecycle is the wrong way to think about order.
In almost every example on your example page, the most recent version status is displayed at the top, and as you go down the list, you get to the least recent version. For example:
or
That pattern breaks in your deprecated in 9.2, removed in 9.5 example:
IMO, it's more useful to follow the previous pattern where the most recent version is always on the top of the list. To accomplish this, we'd either need to (1) adjust this product lifecycle order to something like:
{
[ProductLifecycle.Removed] = new("Removed", "Removed", 0),
[ProductLifecycle.Unavailable] = new("Unavailable", "Unavailable", 1),
[ProductLifecycle.Deprecated] = new("Deprecated", "Deprecated", 2),
[ProductLifecycle.Discontinued] = new("Discontinued", "Discontinued", 3),
[ProductLifecycle.GenerallyAvailable] = new("GA", "Generally available", 4),
[ProductLifecycle.Beta] = new("Beta", "Beta", 5),
[ProductLifecycle.TechnicalPreview] = new("Preview", "Preview", 6),
[ProductLifecycle.Development] = new("Development", "Development", 7),
[ProductLifecycle.Planned] = new("Planned", "Planned", 8),
};
OR (2), sort ordering of lifecycle states by version instead of product lifecycle.
I think option 2 is the correct option as it always ensures that the most recent information is at the top of the list.
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: