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

Update hugo to 0.115.4 #649

Merged
merged 8 commits into from
Aug 1, 2023
Merged

Update hugo to 0.115.4 #649

merged 8 commits into from
Aug 1, 2023

Conversation

jarrodmillman
Copy link
Member

@jarrodmillman jarrodmillman commented Jun 27, 2023

DO NOT MERGE UNTIL UPDATED TRANSLATIONS ARE MERGED

Depends on #639. See #649 (review).

Updating hugo in preparation for updating the theme. There were some changes in hugo 0.112 that are causing issues (addressed in e749f62) : https://gohugo.io/content-management/multilingual/#changes-in-hugo-01120

@netlify
Copy link

netlify bot commented Jun 27, 2023

Deploy Preview for numpy-org ready!

Name Link
🔨 Latest commit 6e686ab
🔍 Latest deploy log https://app.netlify.com/sites/numpy-org/deploys/64c45a76dca30900082709af
😎 Deploy Preview https://deploy-preview-649--numpy-org.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.

@jarrodmillman jarrodmillman requested a review from rgommers June 27, 2023 18:53
@rgommers rgommers requested a review from steppi June 28, 2023 11:09
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @jarrodmillman. These changes all seem useful, but I'd like to make sure that they won't cause problems for the imminent launch of the translated version of the site. We now have fully complete translations, and it seems to me that this may cause significant rework (not sure, I'm hoping @steppi can tell).

If there is a conflict, I'd much prefer to launch the Japanese and Portuguese translations first (should happen very soon), and then rebase this PR on top.

@steppi
Copy link
Contributor

steppi commented Jun 28, 2023

If there is a conflict, I'd much prefer to launch the Japanese and Portuguese translations first (should happen very soon), and then rebase this PR on top.

This branch, hugo-0.114.1, conflicts with the branch l10n_main, containing the updated translations. The conflict is in the files content/ja/config.yaml, content/ja/tabcontents.yaml, content/pt/config.yaml, and content/pt/tabcontents.yaml. As far as I can tell the changes are just a matter of formatting, though there was also some weirdness with duplicate entries.

Currently, l10n_main doesn't build with hugo 0.114.1. I suppose that should be expected @jarrodmillman, and the formatting will need to be updated?

I created a copy of l10n_main and tried to rebase it on hugo-0.114.1, make an effort to resolve the conflicts. When done, I was unable to build the page. I likely made a careless mistake that broke the formatting. The issue though is that Crowdin sometimes isn't very smart about preserving translations when there are updates on main. I'm concerned that if this goes in before the translations, the above files may need to be re-translated.

I think we should launch the translations first; it should be done this week. After that we can update hugo.

@steppi
Copy link
Contributor

steppi commented Jun 28, 2023

I hadn't attempted to build this PR branch locally until just now. I made sure hugo 0.114.1 was installed. The rendered page only includes a top bar with a link to the news page. I've just checked the deploy preview. The same issue appears there as well.

image

@stefanv
Copy link
Contributor

stefanv commented Jun 28, 2023

Hugo 0.114 is forcing us to list non-standard parameters for each language under a params section. If the rendering is incomplete, it sounds like we also need to update the theme to pick up those values from a different location. EDIT: I just checked, and the theme already correctly uses Site.Params to access language-specific parameters.

I think we'll go ahead and fix the theme for now, and then when NumPy is ready (after the language merge?) we can just move the relevant sections under lang: params: ... and update the theme.

I will push a fix to the branch to make the site render correctly.

@jarrodmillman jarrodmillman marked this pull request as draft June 28, 2023 17:04
@stefanv stefanv mentioned this pull request Jul 3, 2023
@steppi
Copy link
Contributor

steppi commented Jul 28, 2023

@jarrodmillman, updated translations are in now.

@stefanv stefanv changed the title Update hugo Update hugo to 0.115.3 Jul 28, 2023
@stefanv stefanv changed the title Update hugo to 0.115.3 Update hugo to 0.115.4 Jul 28, 2023
@stefanv
Copy link
Contributor

stefanv commented Jul 28, 2023

@steppi I hope I managed to preserve all the translation updates!
@jarrodmillman This should now be good to go.

@jarrodmillman jarrodmillman marked this pull request as ready for review July 29, 2023 00:14
Copy link
Contributor

@steppi steppi left a comment

Choose a reason for hiding this comment

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

Looks good. The rendered output for Portuguese and Japanese looks right to me just by comparing with what was there previously, @melissawm @AtsushiSakai. The formatting changes might cause Crowdin to lose translations for the translated yaml files, but if so, I know how to fix that.

@stefanv
Copy link
Contributor

stefanv commented Aug 1, 2023

@steppi If you are happy with this PR, please go ahead and merge it. I'm hesitant, since it may impinge on your work.

@steppi steppi merged commit 5a89146 into numpy:main Aug 1, 2023
@stefanv
Copy link
Contributor

stefanv commented Aug 1, 2023

Specifically, the structure of the config.yaml in #639 will be affected. But the update is also simple.

@stefanv
Copy link
Contributor

stefanv commented Aug 1, 2023

Thanks, let me know if you need a hand in fixing up the others!

@steppi
Copy link
Contributor

steppi commented Aug 1, 2023

Thanks @stefanv. I'm pretty sure this won't cause problems

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

Successfully merging this pull request may close these issues.

4 participants