-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
feat: version callouts #19452
feat: version callouts #19452
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -343,7 +343,7 @@ Setting `NO_COLOR` to anything turns off colorized output, as recommended by | |||
|
|||
### BUILDKIT_HOST | |||
|
|||
_Introduced in [Buildx v0.9.0](../release-notes.md#090)_ | |||
{{< introduced buildx 0.9.0 "../release-notes.md#090" >}} |
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.
This is not rendered because 0.9.0 is below the threshold set in site.Params.min_version_thresholds.buildx
Also generates a warning: https://github.com/docker/docs/actions/runs/7974560715/job/21770734246?pr=19452#step:4:301
@@ -103,8 +103,14 @@ params: | |||
example_alpine_version: "3.19" | |||
example_node_version: "20" | |||
|
|||
min_api_threshold: 1.41 | |||
|
|||
min_version_thresholds: |
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.
Is it n-2 based? (n being the latest stable)
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, these are arbitrary atm so we need to decide. n-2 could be a good default. It might depend what the threshold is depending on the component, e.g. for Desktop n-2 might be too aggressive.
❤️ nice! will have to have a closer look to see how it all works, but ❤️ for working on this! |
The introduced shortcode takes three arguments: - component id (as defined in site.Params) - version (should be semver except for engine api) - link (optional) to e.g. release note The component and version argument is compared against the min threshold set in site config. Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
04c5bf2
to
51790c1
Compare
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.
LGTM
Dreaming a bit here; I wonder if we can combine this in future with some front-end (javascript) code to provide a richer experience;
|
This is really great! Perhaps there's some enhancements to be made for styling; nit-picky me noticed that the indentation feels a bit odd, and that indentation before the One thing I wondered is; if we'll be adding these to reference-docs pages, and if (not sure we do currently?) we use those sources also for generating man-pages; would these be problematic? I see they show up on GitHub (not a huge problem IMO), but for man-pages that could possibly be more problematic. Not sure if there's some syntax possible for them to be hidden (e.g. within html |
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.
oh! so make that an LGTM from my side, but maybe @crazy-max has thoughts on the "man-page" question
@thaJeztah @crazy-max would it makes sense use something like flagset annotations to mark which release a given flag was introduced? e.g. https://pkg.go.dev/github.com/spf13/pflag#FlagSet.SetAnnotation |
Description
Adds an
introduced
shortcode for making version calloutsSupports optional rendering of versions depending on a threshold set in site config.
Also throws a warning if callouts refer to versions below the threshold.
Related issues