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

Use changelogs/release.yaml for the version number source everywhere #3310

Merged
merged 5 commits into from
Aug 10, 2021

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Aug 3, 2021

Currently both the top banner of the site and the top paragraph on the Changelogs page pull the current version number of the spec from config.toml, i.e:

https://github.com/matrix-org/matrix-doc/blob/6c6aa0b3d6eb6edc65c68ad78072a6733998969b/layouts/shortcodes/changelog/changelog-description.html#L8-L15

Note the {{ .Site.Params.version.number }} bit.

However, we already have a method of specifying the version number of a release build of the spec: changelogs/release.yaml:

https://github.com/matrix-org/matrix-doc/blob/6c6aa0b3d6eb6edc65c68ad78072a6733998969b/layouts/shortcodes/changelog/changelog-changes.html#L1-L16

This commit changes the site banner and Changelogs page to use tag in changelogs/release.yaml for the version number of the spec, instead of having two different ways to do things. Note that if we're building an unstable version of the spec, then changelogs/release.yaml is never read from.

Previously both the top banner of the site and the top paragraph on
the changelog page would pull the current version number of the spec
from config.toml.

This commit changes the source of truth to changelogs/release.yaml,
which is already used to determine the current version of the spec
on other parts of the changelogs page.

This also means one less thing to change when doing a release.
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

I'm not really familiar with this templating system, whatever it is - but it looks plausible, as long as you've tested it :)

layouts/partials/navbar.html Outdated Show resolved Hide resolved
layouts/shortcodes/changelog/changelog-description.html Outdated Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Aug 4, 2021

The thought occurs to me: if the unstable flag comes from config.toml, shouldn't the version number? In other words, should we go the other way and make changelog-changes use the value from config.toml rather than release.yaml?

Or should we move the unstable flag into release.yaml?

In short, why the inconsistency?

@anoadragon453
Copy link
Member Author

I'm not really familiar with this templating system, whatever it is - but it looks plausible, as long as you've tested it :)

Oh right - I should've mentioned. It's hugo's own templating system. Looks quite similar to jinja2. https://gohugo.io/templates/introduction/

The thought occurs to me: if the unstable flag comes from config.toml, shouldn't the version number? In other words, should we go the other way and make changelog-changes use the value from config.toml rather than release.yaml?

The historical context is in #2992 (note that we switched from individual release.yaml files to a single one in #2992 after the global version number proposal). I believe the original reason for keeping it in a file versus config.toml was that we originally had to deal with multiple versions of the spec. Nowadays that's not the case.

It would certainly be easier in templates to pull a parameter from the config than parsing yaml files everywhere, so perhaps we should move the date and time into config.yaml. Note that the Changelog page only shows the current release version and the changes since the last release. We just export a different release per git tag, so there'll never be a need for multiple release.yaml files.

@anoadragon453
Copy link
Member Author

@richvdh I've updated things to use the information in config.toml everywhere instead of having a separate changelogs/release.yaml file. Looks good on a test! 🙂

@anoadragon453 anoadragon453 requested a review from richvdh August 10, 2021 15:02
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm otherwise. Presumably this necessitates a change to #3295?

layouts/partials/navbar.html Outdated Show resolved Hide resolved
layouts/partials/navbar.html Outdated Show resolved Hide resolved
@anoadragon453
Copy link
Member Author

Presumably this necessitates a change to #3295?

Yep! Hope you don't mind if I do so via a rebase...

@anoadragon453 anoadragon453 merged commit 0382eac into master Aug 10, 2021
@anoadragon453 anoadragon453 deleted the anoa/single_version_source branch August 10, 2021 17:05
richvdh added a commit that referenced this pull request Aug 23, 2021
richvdh added a commit that referenced this pull request Aug 27, 2021
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.

2 participants