-
Notifications
You must be signed in to change notification settings - Fork 288
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
Hugo docs: Support newer Hugo and Hugo Relearn versions; update README #6264
Hugo docs: Support newer Hugo and Hugo Relearn versions; update README #6264
Conversation
1537c78
to
4400837
Compare
4400837
to
37b8f1e
Compare
Signed-off-by: Bernhard Kaindl <[email protected]>
37b8f1e
to
19f164f
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.
I'm not familiar with the details of Hugo but this looks reasonable to me.
@@ -7,12 +7,21 @@ assetsDir = "assets" | |||
|
|||
[module] | |||
[[module.imports]] | |||
path = 'github.com/McShelby/hugo-theme-relearn' | |||
path = 'github.com/xenserver-next/hugo-theme-relearn' |
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 this intentional?
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.
Yes. It is only an intermediate step in making progress with a small PR.
We could go to Relearn 7 directly on one hop, but then the PR would be far more extensive.
In this case, this intermediate line would not show up in the final change, it would
be reverted to the official Relearn 7 repo in the second part of that future PR.
However, that cumulative PR would be more significant than two smaller step-by-step PRs.
A fuller explanation
This is only a good intermediary step before the next update which then could upgrade the Relearn theme to the current version of Relearn.
I asked Sören Weber (https://github.com/McShelby, the author of Relearn) if he could help. He responded, "The file structure of your repo is uncommon." and linked to one hint. This also suggests that some changes to the structure of the partials might be needed for Relearn 7.
I meanwhile researched the compatibility with upstream Relearn 7 further
According to this, the final update to the current Relearn 7 will need a small structural change as Relearn 7 does not support the architecture of how XenAPI classes and releases are typed, and it suggests that content-generating code should be moved to the content.html partial as documented in the Relearn documentation at https://mcshelby.github.io/hugo-theme-relearn/configuration/customization/partials/index.html
Using this guidance, I've found a working solution for those issues, which, instead of special types for XenAPI classes and releases, which need to be maintained and can no longer include the header.html
and footer.html
(those don't exist in Relearn 7).
Instead, the code in the classes and releases partials should be merged into a content.html
template to generate that content. I have a proof-of-concept: It updates the nearly empty class
and release
files (140 files) and merged the content generation into a new content.html
partial according to the Hugo documentation.
But let's return to this PR, which is just a relatively small second intermediary step:
I solved the issues taken initially during the review by changing the URL of the Relearn module to a fork of Relearn that we can use without having to copy partials from Relearn (like I initially had):
Here are the details
For the details of the issue fixed in the PR and the full update to Relearn 7,
I asked Sören Weber (the author of Relearn) in McShelby/hugo-theme-relearn#1004 if he has some guidance on how to resolve the issue at hand.
He linked to https://mcshelby.github.io/hugo-theme-relearn/configuration/customization/designs/index.html#migration-to-relearn-7-or-higher.
Here is his answer: McShelby/hugo-theme-relearn#1004 (comment)
This solution goes in this direction: It compromises between ease of use (in this case, support for using the Ubuntu snap package and any new build of Hugo in general) and using a moderately new version (or, in this case, fork) of Relearn with the needed fixes.
Conclusion
After solving the compatibility issue and updating Relearn 7.x as described in the README.md, the upstream version will again be used. This is only an intermediate deviation from upstream to facilitate the upgrade.
Alternative
Not making this change would require installing a specific older version (possibly from the source) to generate the docs. This small change is more manageable than installing a special older version of Hugo for development and maintenance until all upgrade problems are fully solved.
Followed up by #6267 |
Minor fixup: It should be ready for a new round reviews now: The incremental change consists only of the required incremental fixes (see the [Compare link](https://github.com/xapi-project/xen-api/compare/e92341cd8439a083e4c8ad54c55690de6f2c04a5..7f64d072899e05f39afe9fd3391bf00ddec2a099) of it. - Review comment from Pau applied: Reverted not running the required pull_request workflows. - Review comment from Pau applied: Fixed not even temporarily using private forks: Squashed commits - While squashing those changes, I: - Split change from `menuTitle` to `linkTitle` into an initial commit (can be done first) - Squashed the changes for `doc/hugo.toml` into one commit. - and extracted the change to `hugo.toml` into one commit (support current Hugo versions) - Squashed the changes for `doc/README.md` into one commit (no intermediate changes) - I found one inadvertent revert of a previous simplification: I removed this accidental revert. ---- Hugo docs: Fix upstream Hugo compatibility: - Includes the fixed commit from #6264, and - Adds a 2nd commit to complete the fix for upstream compatibility. ### Update Hugo version support to work with the current Hugo version This PR fixes using the Ubuntu snap to render the Hugo docs. Relearn 7.x completely REMOVED the `header.html` and `footer.html` partials Partials that useed them are: - https://github.com/xapi-project/xen-api/blob/master/doc/layouts/xenapi/class.html - https://github.com/xapi-project/xen-api/blob/master/doc/layouts/xenapi/release.html To update to 7.x, these had to be moved to the content.html partial, as documented. Changes: - Hugo Relearn 6.x uses the `linkTitle` front matter option now: Quote from the release notes: _The front matter option `menuTitle` [deprecated in 5.24.0](https://mcshelby.github.io/hugo-theme-relearn/introduction/releasenotes/5/index.html#5-24-0) was removed in favour for Hugo’s linkTitle._ - Newer versions of Hugo require the option `unsafe = true` to include raw HTML. - Two Hugo Relearn 6.x partials need fixes for newer versions of Hugo. ### Update doc/README.md accordingly - Update the information on supported versions. - Add information on the breaking changes in Hugo Relearn 7.x.x to watch out for. - While at it, fix/improve the formatting of doc/README.md: - Spelling fixes/improvements - Fix markdown linting errors (empty lines for spacing, no bare links) - Tidy up the document structure
Update Hugo version support to work with the current Hugo version
Follow-up PR: #6267
This PR fixes using the Ubuntu snap to render the Hugo docs.
Changes:
linkTitle
front matter option now:Quote from the release notes: The front matter option
menuTitle
deprecated in 5.24.0 was removed in favor for Hugo’s own linkTitle.unsafe = true
to include raw HTML.Update doc/README.md accordingly