-
Notifications
You must be signed in to change notification settings - Fork 32
Add support for version context variables #1560
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
Conversation
🔍 Preview links for changed docs |
docs/syntax/version-variables.md
Outdated
| | `{{versions.stack.next_major}}` | {{version.stack.next_major}} | The next major version | | ||
| | `{{versions.stack.next_minor}}` | {{version.stack.next_minor}} | The next minor version | | ||
| | `{{versions.stack.base}}` | {{version.stack.base}} | The first version on the new doc system | |
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.
would probably drop the patch segment from these (@lcawl + @colleenmcginnis should confirm they're not needed)
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 might be too preoccupied with consistency, but I'd rather see a consistent pattern across current/next versions. Something like:
| current | next | format |
|---|---|---|
version.stack |
version.stack.next |
#.#.# |
version.stack.major_minor |
version.stack.next_major_minor |
#.# |
version.stack.major_x |
version.stack.next_major_x |
#.x |
version.stack.major_component |
version.stack.next_major_component |
# |
I don't have a strong opinion about versions.stack.base.
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.
version.stack would be version.stack.current but I see your point.
I think {{version.stack.current}} might be too verbose though.
See also my comment on bases here #1560 (comment)
docs/syntax/version-variables.md
Outdated
|
|
||
| | Version substitution | result | purpose | | ||
| |--------------------------------------|-----------------------------------|-----------------------------------------| | ||
| | `{{versions.stack}}` | {{version.stack}} | Current version | |
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.
in most cases I think we want the base usage to be the major_minor version. might want to make that the "core" and make a versions.stack.patch or something instead
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.
Should it be formatted e.g 9.0.0 = 9.0 and 9.0.1 would be 9.0.1 ?
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 not sure about that. I think so though?
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 agree that the installation instructions would need the full V.R.M but that would not be the general case.
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.
in most cases I think we want the base usage to be the major_minor version.
I agree that the installation instructions would need the full V.R.M but that would not be the general case.
Installation instructions and download links are probably the most important use cases for me. 😬
Should it be formatted e.g 9.0.0 = 9.0 and 9.0.1 would be 9.0.1 ?
I think I'd prefer it to be major-minor-patch if only for consistency.
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 also think the path of least surprise is to have the base be full V.R.M
{{versions.stack}}{{versions.stack.base}}{{versions.stack.next_major}}{{versions.stack.next_minor}}
Where the V.R and V.x and V variants can all be obtained using a *.suffix.
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 opened #1563 as a completely new approach that allows a more generic approach to reformatting variables.
In that PR we only expose:
{{versions.stack}}{{versions.stack.base}}
With operators to get a specific format e.g
{{versions.stack | M.x }}
| key = $"version.{name}.next_minor"; | ||
| _substitutions[key] = new SemVersion(current.Major, current.Minor + 1, current.Patch, current.Prerelease, current.Metadata); | ||
|
|
||
| key = $"version.{name}.next_major"; | ||
| _substitutions[key] = new SemVersion(current.Major + 1, current.Minor, current.Patch, current.Prerelease, current.Metadata); |
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.
Are these actually required?
I'm afraid that these variables will cause confusion and will output an unexpected version if used wrongly by the user.
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 can't think of a use case where you want to have a next version that changes over time.
Can you explain the use case you were thinking of?
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.
@lcawl and wider @elastic/docs-tech-leads can you confirm whether we have a use-case for next major/minor as variables?
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.
Also keen to hear thoughts on {{versions.*}} vs {{version.*}} :)
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.
These are no longer exposed directly but can be obtained through variable mutation operators. See #1563
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.
Sticking with {{version.*}} for now.
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.
As per my comment here: #1560 (comment) next major and minor are not exposed through variables but can be obtained using variable mutations.
Co-authored-by: Jan Calanog <[email protected]>
Co-authored-by: Jan Calanog <[email protected]>
* Add support for variable operator syntax
* do not report version variables as unused
* Allow a space after {{
* carry trimming over to split
* Temporarily make variables with spaces that are not found hints, we used to not do anything here
* flip ternary
Version are exposed during build using the {{versions.VERSIONING_SCHEME}} variable.
For example stack versioning variables are exposed as {{versions.stack}}. As well as offering several specialized suffixes to expose components or next version numbers.