Skip to content
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

Improve patch releases page #46368

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented May 14, 2024

Improve https://kubernetes.io/releases/patch-releases/ especially around date rendering.

  • Here's a preview link for English.
  • Here's a preview link for Bangla / Bengali.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/release-eng Issues or PRs related to the Release Engineering subproject labels May 14, 2024
@k8s-ci-robot k8s-ci-robot added area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 14, 2024
Copy link

netlify bot commented May 14, 2024

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 3ab2d3a
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/6713a02b3ab72d00080d4e32
😎 Deploy Preview https://deploy-preview-46368--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -519,12 +519,27 @@ other = "2006-01-02"
[release_date_format_month]
other = "January 2006"

# Use 2006-01-02 (ISO 8601) if you are not sure what to use.
[release_date_format_text]
other = "Monday, 02 Jan 2006"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will work for localization teams.
This encoding is Golang specific, and the convention here won't translate well into any other languages.

ISO 8601 is a standard that can be followed by all localization teams though.

Copy link
Contributor Author

@sftim sftim May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Use 2006-01-02 (ISO 8601) if you are not sure what to use.

I could change that to explicitly mention that Hugo doesn't support localized date formatting. For English, we can still provide the tooltip, and if Hugo gains support, this will Just Work for other locales.

How about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can localize dates, but we have to use the exact language codes.
https://discourse.gohugo.io/t/localizing-date-format/37868 has more on this.

data/i18n/en/en.toml Show resolved Hide resolved
<td>
{{ time.Format ( T "release_date_format") $patchReleaseInfo.targetDate }}
{{- with $patchReleaseInfo.targetDate -}}
<time title="{{ time.Format ( T "release_date_format_text") . }}" datetime={{ time.Format "2006-01-02" . }}>{{ time.Format ( T "release_date_format" ) . }}</time>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned with the direction of this change is leading us to.
There is no doubt that the automated generation of certain pages or elements could help improve productivity. But this is still a documentation project anyway. Over engineering may not be a good thing to website maintainers. Whenever possible, we should try "keep it simple and stupid (aka KISS)". Carving on shit is not something we as a community should encourage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The patch releases page is one of our most frequently updated pages and largely changes through data-driven changes.
  • Rather than improve productivity for SIG Release, my intent here is to improve UX for people reading the rendered page.
    For example, if you can see that an EOL date is a date your business does not usually work, that helps you plan.
    It's even better for firms not to wait until the last possible moment to make a change, but I know there are corporations that wait exactly that long.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested an alternative, more complicated approach: #46426

@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from 241ec56 to b58fe0b Compare May 29, 2024 08:55
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from b58fe0b to 3b5e7f8 Compare June 20, 2024 15:00
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from 3b5e7f8 to 8d7cdc7 Compare July 30, 2024 12:37
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from 8d7cdc7 to 6b228d7 Compare August 19, 2024 10:26
@sftim
Copy link
Contributor Author

sftim commented Aug 19, 2024

I think this is ready for review. Please take a look, folks.

@sftim sftim marked this pull request as ready for review August 19, 2024 10:27
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 19, 2024
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from 6b228d7 to f3f8adc Compare September 23, 2024 12:00
- endOfLifeDate: "2021-05-12"
finalPatchRelease: 1.18.19
note: '[Regression](https://groups.google.com/g/kubernetes-dev/c/KuF8s2zueFs)'
note: >-
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this change, can you please clarify it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also getting this diff when running schedule-builder:

diff --git a/data/releases/eol.yaml b/data/releases/eol.yaml
index 739b542b3f..e28b84f20a 100644
--- a/data/releases/eol.yaml
+++ b/data/releases/eol.yaml
@@ -48,9 +48,7 @@ branches:
   release: "1.19"
 - endOfLifeDate: "2021-06-18"
   finalPatchRelease: 1.18.20
-  note: >-
-    Release 1.18.20 was created to solve a
-    [regression](https://groups.google.com/g/kubernetes-dev/c/KuF8s2zueFs)
+  note: Release 1.18.20 was created to solve a [regression](https://groups.google.com/g/kubernetes-dev/c/KuF8s2zueFs)
     introduced in 1.18.19
   release: "1.18"
 - endOfLifeDate: "2021-01-13"
 -

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention making changes to this file was to fix some issues about how well the data fitted what the website needs - see 633460f

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops; when I started this PR, the file didn't have the helpful comment about being autogenerated.

Can we still fix v1.18 having two EOL dates?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, fixing that is fine, I also left a comment about fixing it for the v1.22 release.

@asem-hamid
Copy link
Contributor

image

image

There is a small bug here: the month name is not being localized in the Monthly Patch Release column. However, it works properly in the Cherry Pick Deadline and Target Date columns.

@sftim
Copy link
Contributor Author

sftim commented Sep 23, 2024

@asem-hamid would you be willing to disclose the URL or URL path for the page that you think isn't right?

BTW, this might be a bug in Hugo (I'm not yet sure).

I assume that this is the patch releases page for Bangla? If so, it's not a regression; https://kubernetes.io/bn/releases/patch-releases/ is already buggy. See #48057

@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from f3f8adc to 20e9d32 Compare September 23, 2024 18:03
Comment on lines -53 to -56
- endOfLifeDate: "2021-05-12"
finalPatchRelease: 1.18.19
note: '[Regression](https://groups.google.com/g/kubernetes-dev/c/KuF8s2zueFs)'
release: "1.18"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed because having two v1.18 EOL dates is unhelpful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sftim To make it consistent, can you please do the same for the v1.22 release?

@@ -50,10 +50,6 @@ branches:
finalPatchRelease: 1.18.20
note: Created to solve regression introduced in 1.18.19
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: this is not a note about v1.18; there's some challenge here about how we explain the available notes to readers.

Another PR can fix that, I hope.

@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch 2 times, most recently from 75b45a7 to 26f69d7 Compare September 26, 2024 10:05
# Used if the maintenance mode date is in the past, otherwise
# see release_maintenance_and_end_of_life_details_current
[release_maintenance_and_end_of_life_details_past]
other = """🛈 **Kubernetes {{ .minor_version }} entered maintenance mode on {{ .maintenance_mode_start_date }}**.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this character at the beginning of the sentence? It's showing as a box in GitHub preview, and doesn't show at all on the website.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's similar to ℹ️

I can switch it to something more widely supported.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be great, other than that, the PR looks good to me!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(done)

@xmudrii
Copy link
Member

xmudrii commented Sep 30, 2024

@sftim Is it possible to make all tables wider, potentially taking all the available width? For example, we have such a table in the CVE feed. Ideally, that space would be used for the notes.

@sftim
Copy link
Contributor Author

sftim commented Sep 30, 2024

@sftim Is it possible to make all tables wider, potentially taking all the available width? For example, we have such a table in the CVE feed. Ideally, that space would be used for the notes.

Maybe, but that's best left for another PR.

@sftim
Copy link
Contributor Author

sftim commented Sep 30, 2024

If we want a better list of minor / patch releases, see #46426
Render list of releases using a content adapter

We'd end up with a page per [something]. Eg a page (and URL) per minor release, and a table that links to sections within the right minor release page.

@xmudrii
Copy link
Member

xmudrii commented Sep 30, 2024

@sftim Sounds good, I left a comment on #46426

sftim added 2 commits October 19, 2024 13:02
We had two EOL dates for v1.18, and three for v1.22, which was likely to confuse
readers more than it helps them. Fix that.
and link to CVE list
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from 26f69d7 to fb19c41 Compare October 19, 2024 12:02
- Use Hugo's built-in support for localizing dates
- Allow customizing date formats separately
- Make more dates machine readable
- Add a tooltip for dates
@sftim sftim force-pushed the 20240514_improve_patch_releases_page branch from fb19c41 to 3ab2d3a Compare October 19, 2024 12:03
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR, @sftim!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 21, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: af8e0665e923d328ecbb11c3e406eaaff9f166dc

@sftim
Copy link
Contributor Author

sftim commented Oct 21, 2024

#46368 (comment)

Given this has an approval from a SIG Release approver, then even though it is a change to the live site:
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim, xmudrii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 21, 2024
@k8s-ci-robot k8s-ci-robot merged commit f962ee6 into kubernetes:main Oct 21, 2024
6 checks passed
@sftim sftim deleted the 20240514_improve_patch_releases_page branch October 21, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. sig/release Categorizes an issue or PR as relevant to SIG Release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants