-
Notifications
You must be signed in to change notification settings - Fork 32
Introduce substitutions for previous versions #2111
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
🔍 Preview links for changed docs |
Mpdreamz
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.
I am not in love with with the previous moniker.
Could this be a mutation? e.g
{{ .elasticsearch | version_legacy }}
Ideally we can support version and base also as mutations
{{ .elasticsearch | version }}
{{ .elasticsearch | version_base }} ?
|
|
||
| foreach (var (id, system) in versionsConfig.VersioningSystems) | ||
| foreach (var (id, system, previousVersion) in | ||
| versionsConfig.VersioningSystems.Select(kvp => |
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.
Ensure VersioningSystems is correct and we can read it.
e.g add system.Legacy as an additional property so we can keep on simply itterating on VersioningSystems
likewise, while I appreciate that this is a straightforward way to make this change, it's not going to be robust long term (i.e. once we hit a new major). the info we actually want here is "the last minor of the previous major", which will break down when we move to stack 10 and 9.x docs are accessible in a way other than traversing mapped pages. I'd rather just see this as another piece of info stored in versions.yml ... we can avoid adding it until it's needed, product by product. alt: we just store this as a sub in docset.yml. again, we don't need this for all products afaik (maybe @colleenmcginnis can correct me if she sees a big need for this in ingest tool land) |
Nope. I don't see any need for this in ingest docs. For the most part we just refer to 8.19 (minor only). |
|
Finally, this actually doesn't do what I think @eedugon was asking for. I think edu wants to specify down to the patch version, and this just pulls the minor because it's using the mapped page info, which is not aware of the patch version. |
|
I agree with @shainaraskas , we need something that substitutes to Getting |
|
I see. Yeah, the need for the patch version means the initial premise here of picking this information from legacy url mappings isn't good enough. We can add this information to |
|
@cotti I think this is mostly a nice to have feature but not critical. might be worth doing if it's trivially easy |
|
@cotti , @shainaraskas : if we want to automate this and go to the source of truth of that version I'd say that we can get it from the legacy The file And it would be automatically updated after every release :) So if we can make docs-builder to fetch it we wouldn't need to manually touch docs-V3 after releasing a 8.19 version. But keeping it in versions.yaml and just manually updating it after every 8.19 release is also a good approach, happy with that also. |
Closes #2092