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

Provide MD checklists for the changelog #1937

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

Johennes
Copy link
Contributor

@Johennes Johennes commented Aug 22, 2024

Fixes: #1682

This changes /changelog to be a section and adds a separate output format to provide Markdown checklists under /changelog/$VERSION/checklist.md.

Example:

I've also added links to the checklists in each version's header table.

Screenshot 2024-09-09 at 09 33 35

Since each version's changelog has a separate page now, inlining all changelogs on the index page seemed superfluous. Similarly, having the index page only list historical spec versions felt odd. Therefore, I've moved the info about historical versions to its own page and made the index page redirect to the latest version's changelog page.

Screenshot 2024-09-09 at 09 34 37

Pull Request Checklist

Preview: https://pr1937--matrix-spec-previews.netlify.app

Fixes: matrix-org#1682
Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>
@Johennes Johennes changed the title Output changelog checklists Output changelog checklists (Variant 2) Aug 22, 2024
@Johennes Johennes marked this pull request as ready for review August 23, 2024 14:23
@Johennes Johennes requested a review from a team as a code owner August 23, 2024 14:23
@Johennes Johennes changed the title Output changelog checklists (Variant 2) Provide MD checklists for the changelog Aug 23, 2024
@zecakeh
Copy link
Contributor

zecakeh commented Sep 4, 2024

It seems strange to me to have the same content twice in "Changelog" and in the different versions. This also complicates the rendering because the levels of the headings should be different in both cases.

Maybe clicking on "Changelog" could just open the latest version?

@Johennes
Copy link
Contributor Author

Johennes commented Sep 4, 2024

Maybe clicking on "Changelog" could just open the latest version?

I like that, yeah. I was worried that changing that page would break existing links. However, I'm now realizing any existing link (such as https://spec.matrix.org/v1.11/changelog/#internal-changestooling) would point into a historic spec version anyway. So this seems like a non-issue. 🤦

@Johennes
Copy link
Contributor Author

Johennes commented Sep 9, 2024

Maybe clicking on "Changelog" could just open the latest version?

Have made it so.

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've removed this entirely because the added info felt superfluous. Every changelog lists changes made since the last release and the version is apparent via the URL and page heading.

scripts/generate-changelog.sh Show resolved Hide resolved
scripts/generate-changelog.sh Outdated Show resolved Hide resolved
@zecakeh
Copy link
Contributor

zecakeh commented Sep 10, 2024

Thanks, it looks good. I like that when we hover a version in the sidebar, there's the full title that appears.

I am not a fan of the redirect on the "Changelog" page, I would prefer for the link to be the correct one, but I guess there is no simple way to do that with Hugo.

@Johennes
Copy link
Contributor Author

I am not a fan of the redirect on the "Changelog" page, I would prefer for the link to be the correct one, but I guess there is no simple way to do that with Hugo.

Yeah, it's not pretty, especially because the index page briefly flashes before redirecting. Maybe we could render the latest version's changelog inline on the index page instead? That would effectively make /changelog a shortcut for /changelog/$latest.

@richvdh richvdh self-requested a review September 25, 2024 10:52
Comment on lines +6 to 8
{{ with index .Page.RegularPages.ByDate.Reverse 0 }}
<meta http-equiv="refresh" content="0; url={{ .Permalink }}" />
{{ end }}
Copy link
Member

Choose a reason for hiding this comment

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

we could add something like:

manualLinkRelRef: unstable.md

to the front matter of content/changelog/_index.html, but it would need maintaining during releases. (We could hack the generate-changelog.sh script to update it?)

Or we could add more intelligence to layouts/partials/sidebar-tree.html to handle this case?

This is probably fine for now, and we could improve it in a future PR.

@richvdh
Copy link
Member

richvdh commented Sep 27, 2024

I'm inclined to merge this if there are no pressing objections from other SCT members.

@richvdh
Copy link
Member

richvdh commented Sep 27, 2024

(I like breaking the changelog up into pages, irrespective of any benefits from the checklist)

@clokep
Copy link
Member

clokep commented Sep 27, 2024

I'm a fan of the output, but haven't checked the implementation.

@richvdh
Copy link
Member

richvdh commented Sep 27, 2024

Let's ship it then

@richvdh richvdh merged commit 00af39e into matrix-org:main Sep 27, 2024
12 checks passed
@anoadragon453
Copy link
Member

This appears to have generated broken links on the live site: https://github.com/matrix-org/matrix-spec/actions/runs/11070201829/job/30759445748#step:6:13

@Johennes
Copy link
Contributor Author

Hm, looks like these are from links in the header:

<link rel="alternate" type="text/markdown" href="[/unstable/unstable/changelog/v1.5/checklist.md](view-source:file:///unstable/unstable/changelog/v1.5/checklist.md)">

Locally these look fine for me though:

<link rel="alternate" type="text/markdown" href="[//localhost:1313/changelog/v1.5/checklist.md](view-source:http://localhost:1313/changelog/v1.5/checklist.md)">

🤔

@Johennes
Copy link
Contributor Author

I've opened #1954 to remove these links entirely.

@zecakeh
Copy link
Contributor

zecakeh commented Sep 30, 2024

It's not the only URLs that are affected, the one from the <meta http-equiv="refresh" …/> is too, and is more problematic. I believe that changing .Permalink to .RelPermalink in layout/shortcodes/changelog/changelogs.html fixes it though.

@zecakeh
Copy link
Contributor

zecakeh commented Sep 30, 2024

Basically, the problem seems to be the use of canonifyURLs: true with --baseURL. The former is deprecated so we should probably work on removing it (just removing it breaks some URLs right now).

@Johennes
Copy link
Contributor Author

It's not the only URLs that are affected, the one from the <meta http-equiv="refresh" …/> is too, and is more problematic. I believe that changing .Permalink to .RelPermalink in layout/shortcodes/changelog/changelogs.html fixes it though.

Ah, I missed this. Thanks @zecakeh! Looks like .RelPermalink fixes this, indeed. I've opened #1956.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider publishing a pre-made Markdown checklist alongside changelog
5 participants