Add development tooling and QA infrastructure#10856
Add development tooling and QA infrastructure#10856timlinux wants to merge 4 commits intoqgis:masterfrom
Conversation
Build System: - Add timing output to Makefile targets showing duration - Add parallel build support with -j auto - Add fasthtml, draft, serve targets for development Development Environment (flake.nix): - Add codespell, yamllint, doc8 for linting - Add shell hook with command reference table - Add nix run targets for all build and utility commands Pre-commit (.pre-commit-config.yaml): - Add trailing-whitespace, end-of-file-fixer - Add check-yaml, check-json, check-merge-conflict - Add rstcheck, doc8, codespell, yamllint - Add custom check-image-refs script GitHub Actions: - Add qa.yml workflow using Nix for reproducible CI - Add optimize-images.yml for PNG optimization Neovim Integration: - Add .nvim.lua with which-key project menu - Add .exrc with quick access shortcuts Scripts: - benchmark_build.sh - performance comparison - build_all_languages.sh - parallel i18n builds - optimize_images.sh - PNG optimization - check_image_refs.sh - validate image references
pre-commit run --all-files
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook
Fixing docs/user_manual/processing/console.rst
Fixing docs/training_manual/spatial_databases/spatial_functions.rst
Fixing docs/user_manual/working_with_projections/working_with_projections.rst
Fixing docs/documentation_guidelines/first_contribution.rst
Fixing docs/user_manual/processing_algs/qgis/cartography.rst
Fixing docs/user_manual/processing_algs/qgis/database.rst
Fixing docs/user_manual/processing/configuration.rst
Fixing docs/pyqgis_developer_cookbook/plugins/snippets.rst
Fixing docs/training_manual/map_composer/map_composer.rst
Fixing docs/user_manual/appendices/qgis_r_syntax.rst
Fixing docs/user_manual/working_with_vector_tiles/vector_tiles.rst
Fixing docs/training_manual/vector_classification/classification.rst
Fixing docs/user_manual/processing_algs/qgis/interpolation.rst
Fixing docs/training_manual/processing/more_backends.rst
Fixing docs/user_manual/plugins/core_plugins/plugins_metasearch.rst
...
...
...
Found 21 missing image(s) in 56 reference(s)
@selmaVH1 and @hefniraera it would be great if you can adopt a zero warnings,zero errors policy |
Thanks @timlinux we will try, this looks nice. Should we fix those image references before this PR gets merged? |
hi @selmaVH1 no there are many many other errors (I had to truncate the listing because it was too long for github). I suggest to merge this first and then we work through the list of errors until thee are none left. The qa checks I made will fail if you make changes to a file and it has new or pre-existing errors, but will pass if a file was unchanged and has a pre-existing error. I added some other tooling that willl let me quickly work through the errors, but it uses vim so it might be better to let me look at it. note that it is going to be a very wide reaching change, touching many file. |
Thanks for the explanation @timlinux. Let's see if @DelazJ has some comments, before merging. |
- Fix regex to handle absolute paths with colons - Filter out .direnv paths (not part of project) - Remove duplicate entries - Improve notification messages
Pre-commit QA SummaryHook Status Overview
Error Counts by Type
Summary
Recommended Priority
|
DelazJ
left a comment
There was a problem hiding this comment.
Thanks @timlinux for trying to automate the process
Well, it took some time to review and TBH I do not understand half of the code here (or the use case is not obvious to me. Yet.) I anyway tried to walk through these and alert on some orientation that look not that right to me. Hopefully it makes sense.
One more step to add to the plan: Over the years, we have tried to create small scrpts to address some automations. I'd like that these changes are not one layer on top of the existing process but that they are merged together, cleaned up and documented so that most of the interested parties can understand what they can do or no, how and when.
PS: colored and commented output is really nice.
| echo -e "$(CYAN) Building HTML documentation ($(LANG)) - fast mode$(NC)"; \ | ||
| echo -e "$(BLUE)━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━$(NC)"; \ | ||
| echo ""; \ | ||
| $(SPHINXBUILD) -b html -j auto "$(SOURCEDIR)" "$(BUILDDIR)/html/$(LANG)" $(SPHINXOPTS) $(0); \ |
There was a problem hiding this comment.
Do we need (here and below) the -j auto if $(SPHINXOPTS) is also passed?
|
|
||
| # Quick parallel HTML build (subset of languages for testing) | ||
| i18n-quick: | ||
| ./scripts/build_all_languages.sh --html --languages "en,de,fr" |
There was a problem hiding this comment.
| ./scripts/build_all_languages.sh --html --languages "en,de,fr" | |
| ./scripts/build_all_languages.sh --html --languages "en,nl,fr" |
Because that one is the most regular translated language we have in docs
| TX_LANGS="" | ||
| for lang in "${LANG_ARRAY[@]}"; do | ||
| if [[ "$lang" != "en" ]]; then | ||
| tx_lang="${lang//_/-}" # Replace underscore with dash for Transifex |
There was a problem hiding this comment.
Unless I misunderstand, this logic does not look corret to me. Transifex uses zh-Hans while Sphinx uses zh_Hans but both uses pt_BR or Pt_PT (to name languages we build), keeping the underscore. So pt_BR would become pt-BR?
IIRC, the zh mismatch is currently handled in the .tx/config file so I'm not sure we need this trick to load the translation to the correct place. Where things get complicated is that we expose zh-Hans in the docs drop-down menu and we need to relink in order to use translation shortcut from HTML docs
| JOBS=$(nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4) | ||
|
|
||
| # Languages from Makefile (excluding en for translation pull) | ||
| ALL_LANGUAGES="en de es fr it ja ko nl pt_BR pt_PT ru zh_Hans" |
There was a problem hiding this comment.
This list does not seem up to date with current Makefile. And I see it is reused in a few places. Is there no way to have it in a single place? And is it something docs writers have to manage?
Further notes:
- master branch has English only exposed (and IS NOT connected to Tx) so running this on master would just duplicate English docs (using the few translated images)
- release_* branches have their translations (and release_3.44 is the only branch connected to Tx)
There was a problem hiding this comment.
Running the script on master, the report looks a bit weird, reporting failing builds while all the languages are built (in English) and no specific errors reported in the logs.
Details
./scripts/build_all_languages.sh --html═══════════════════════════════════════════════════════════════
QGIS Documentation Multi-Language Build
═══════════════════════════════════════════════════════════════
Configuration:
Languages: en de es fr it ja ko nl pt_BR pt_PT ru zh_Hans
Parallel jobs: 8
Pull translations: false
Build HTML: true
Build PDF: false
Clean first: false
═══════════════════════════════════════════════════════════════
Building HTML Documentation (Parallel)
═══════════════════════════════════════════════════════════════
Building html for de...
Building html for en...
Building html for es...
Building html for fr...
Building html for it...
Building html for nl...
Building html for ko...
Building html for ja...
✓ en (html) completed
Building html for pt_BR...
✓ ko (html) completed
Building html for pt_PT...
✓ es (html) completed
Building html for ru...
✓ de (html) completed
Building html for zh_Hans...
✓ nl (html) completed
✓ it (html) completed
✓ fr (html) completed
✓ ja (html) completed
✓ ru (html) completed
✓ pt_BR (html) completed
✓ pt_PT (html) completed
✓ zh_Hans (html) completed
HTML builds completed
Output directories:
build/html/en (303M)
build/html/de (303M)
build/html/es (304M)
build/html/fr (303M)
build/html/it (303M)
build/html/ja (301M)
build/html/ko (303M)
build/html/nl (298M)
build/html/pt_BR (302M)
build/html/pt_PT (302M)
build/html/ru (302M)
build/html/zh_Hans (301M)
═══════════════════════════════════════════════════════════════
Build Complete
═══════════════════════════════════════════════════════════════
Total time: 7m 27s
Languages built: 12
Failed builds:
✗ en (html)
✗ de (html)
✗ es (html)
✗ fr (html)
✗ it (html)
✗ ja (html)
✗ ko (html)
✗ nl (html)
✗ pt_BR (html)
✗ pt_PT (html)
✗ ru (html)
✗ zh_Hans (html)
Check logs in build/logs/ for details
|
|
||
| # Build language list for tx (excluding en, replacing zh_Hans with zh-Hans) | ||
| TX_LANGS="" | ||
| for lang in "${LANG_ARRAY[@]}"; do |
There was a problem hiding this comment.
This would pull languages only listed in the Makefile, right? What about the other languages in tx? Currently, we pull all of them regularly (1), and especially when switching versions to make sure that each version stored translation efforts in it. Also a good way to evaluate whether a new language should be released.
We pull everything, we build Makefile only!
(1) This is supposed to be done with pull_minimized_translations.yml but it never pulls all the expected strings
| if [[ -n "$TX_LANGS" ]]; then | ||
| echo -e "Pulling languages: ${YELLOW}$TX_LANGS${NC}" | ||
| if [[ "$DRY_RUN" == true ]]; then | ||
| echo "Would run: tx pull -f --parallel --mode onlytranslated -l $TX_LANGS" |
There was a problem hiding this comment.
QGIS docs have >60 target languages in Transifex, we do publish docs for ~10 that reached 5% of actual translation, meaning that most of the languages (including the ones we build may) have very few translation.
The --onlytranslation mode afaict pulls more strings than necessary and I've tried to circumvent that in scripts/minimize_translations.sh and avoid bloating the repository. I'd appreciate if we could consider keeping that cleanup.
There was a problem hiding this comment.
Is there a chance we include in the checks:
- linkcheck
- rst formatting strings I tried to cover in strings_fixer.sh. In a nicer way...
|
|
||
| on: | ||
| push: | ||
| branches: [master, main] |
There was a problem hiding this comment.
| branches: [master, main] | |
| branches: [master] |
no main afaict
| set tabstop=3 | ||
| set shiftwidth=3 | ||
| set softtabstop=3 | ||
| set textwidth=79 |
There was a problem hiding this comment.
Is this for the max length of a line in rst file, Or for dev interface?
A little bit of context for this PR:
We are holding an infrastructure sprint this week and the documentation build and publish pipeline is going to be one of our focus areas. In particular the whole process for generating the docs is quite inefficient in terms of server deployments. We also don't have a guarantee that each PR merged does not break build pipelines so we want to fix that.
Summary
This PR adds comprehensive development tooling and QA infrastructure to improve the documentation workflow.
Build System Improvements
-j autofor faster buildsfasthtml,draft,servefor development workflowDevelopment Environment (Nix flake)
nix developnix runtargets for all build and utility commandsPre-commit Checks
GitHub Actions
qa.yml- Comprehensive QA workflow using Nix for reproducible CIoptimize-images.yml- PNG optimization for PRs and manual runsNeovim Integration
.nvim.luawith which-key project menu (<leader>p).exrcwith quick access shortcutsScripts
benchmark_build.sh- Compare single-core vs multi-core performancebuild_all_languages.sh- Parallel i18n buildsoptimize_images.sh- PNG optimization (lossless/lossy)check_image_refs.sh- Validate image references existTest plan
make fasthtmland verify timing output displayspre-commit run --all-filesto test hooksLLM assistants helped in the making of this PR