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

Specify site_url in MkDocs configuration for correct sitemap #1054

Merged
merged 1 commit into from
Dec 13, 2022

Conversation

StevenMaude
Copy link
Contributor

@StevenMaude StevenMaude commented Dec 8, 2022

Fixes #1053.

To correctly generate the sitemap.xml, the site_url must be specified in mkdocs.yml.

Having a valid sitemap can help search engines index a site's content.

What I've done here is to make site_url switchable depending on an environment variable, MKDOCS_SITE_URL and then simply set that on build. As this is only really required for deployment, I've just put this in the deployment workflow, instead of adding some wrapper or to do the build with the environment variable supplied.

This keeps the local development setup exactly as it was, with no value set for site_url.

I'm reasonably convinced that this is the right approach because:

  • we can override the site_url
    • to set as an empty string if we ever want to build a local file distribution (this may not be necessary with a missing value, but not tested)
    • when running locally with no URL set, the configuration is as it was previously (although I think with mkdocs serve, the site URL is ignored anyway!)
  • we probably do want the canonical URL to be https://docs.opensafely.org/some-path… even for Cloudflare test deployments, technically, should a search engine somehow stumble upon those pages

To be able to correctly produce `sitemap.xml` and set correct canonical
link tags.
@StevenMaude
Copy link
Contributor Author

I'm reasonably convinced that this is the right approach…

…but am happy to be persuaded otherwise, that, for instance, we should only set site_url for actual production deployments to the site (and not for other Cloudflare Pages previews that we get)

Copy link
Contributor

@inglesp inglesp left a comment

Choose a reason for hiding this comment

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

Looks reasonable.

@StevenMaude StevenMaude merged commit 15edbf7 into main Dec 13, 2022
@StevenMaude StevenMaude deleted the steve/sitemap-generation branch December 13, 2022 10:03
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.

sitemap.xml not correctly generated for production site
2 participants