A generic workflow to build Sphinx documentation and deploy to gitHub Pages

[Yaswant Pradhan (yaswant)]

PR Summary

Code Reviewer: James Bruten (@james-bruten-mo)

closes #34

A generic reusable workflow to build and deploy sphinx-based documentation with minimal effort.

Code Quality Checklist

• I have performed a self-review of my own code
• My code follows the project's style guidelines
• The modified workflow's README has been updated, if required
• The changes have been sufficiently tested (see https://github.com/MetOffice/git_playground/pull/134)

AI Assistance and Attribution

• Some sections of the sphinx-docs/README.md were enhanced with assistance from Met Office Github Copilot Enterprise and I have followed the Simulation Systems AI policy

Code Review

• The changes are approriate and testing has been sufficient

[Andrew Coughtrie (andrewcoughtrie)]

Looks good on the whole, a couple of suggestions to help with debugging etc.

[Andrew Coughtrie (andrewcoughtrie)]

This all looks good to me now.

[Andrew Coughtrie (andrewcoughtrie)]

James Bruten (@james-bruten-mo) do you want to have a look at this as well?

[James Bruten (james-bruten-mo)]

I've got one minor change request to rename one of the variables pushed to github_env.

I guess I'm not completely convinced by this as a change. In the repos we manage, we'll be replacing a handful of relatively simple workflows with a single huge and complicated ones - I'm not sure that in this case, we're increasing maintainability?
I also really don't like the use of python throughout. Effectively we've got inline python, within a bash script, within a yaml file, which is something that you can do, but I'm not convinced we should. I'm not suggesting that it be rewritten as bash, but maybe it'd be better storing in a separate python file and just calling that?

The workflow seems reasonably impressive in many ways - I just worry that it's a little too complicated to be increasing maintainability (not to mention if it does break, all repos will be affected rather than just one at the moment). If you want to keep pursuing, then sure, but just noting some concerns.

[James Bruten (james-bruten-mo)]

I suggest found should be something more explicit here, found_makefile if nothing else

[Yaswant Pradhan (yaswant)]

done.

[Yaswant Pradhan (yaswant)]

I've got one minor change request to rename one of the variables pushed to github_env.

I guess I'm not completely convinced by this as a change. In the repos we manage, we'll be replacing a handful of relatively simple workflows with a single huge and complicated ones - I'm not sure that in this case, we're increasing maintainability? I also really don't like the use of python throughout. Effectively we've got inline python, within a bash script, within a yaml file, which is something that you can do, but I'm not convinced we should. I'm not suggesting that it be rewritten as bash, but maybe it'd be better storing in a separate python file and just calling that?

The workflow seems reasonably impressive in many ways - I just worry that it's a little too complicated to be increasing maintainability (not to mention if it does break, all repos will be affected rather than just one at the moment). If you want to keep pursuing, then sure, but just noting some concerns.

I understand your views and myself thought about keeping the scripts part separate (e.g., .github/scripts). But that will make the use of reusable workflow more complex when called from a forked PR unless we explicitly handle the checkout process . Keeping the messy python bit inline is guaranteed to work without additional work.

If you could test a few edge cases where you think this might fail, that would be great

[Yaswant Pradhan (yaswant)]

James Bruten (@james-bruten-mo) thanks for the review and for some critical points James raised. It made me think again! So, I've made some changes to address some concerns:

1. The dependency resolution using conf.py was too long, too nested, a maintenance overhead. I've simplified this - if you are a fan of list comprehension you will appreciate the changes. This will resolve all dependencies we have in any SimIT documentation repositories.
   However, separating this as a standalone script will be counter-productive and will require checking out the script explicitly (unnecessary overhead + dependency).
2. Replaced the toml parsing from python to yq (for yaml, but can handle toml, one-way) - I thought it was not available on action runner. Turns out its now available on ubuntu-* and macos runners. So if you are happy with the changes, I'll update the README to indicate that this is not 100% compatible on Windows runners or add an extra step to install if runner is Windows (for the sake of completeness)..

Andrew Coughtrie (@andrewcoughtrie) thanks for the discussion around ubuntu-slim runner offline. This is a perfect choice for sphinx-build on our private repositories.

I've tested few configurations here: https://github.com/MetOffice/git_playground/pull/136

[James Bruten (james-bruten-mo)]

Thanks Yash, that looks better with the reduced python. Do you want to update the readme - I don't think we need to worry about windows runner support