glaterza commented on PR #44395: URL: https://github.com/apache/superset/pull/44395#issuecomment-5784818096
Rebased onto current `master` after #44467, which regenerated `messages.pot` and added the drift guard. There are three changes since the last review. **The first is a new fix and needs a fresh look.** **1. New fix: the trailing-line strip (a9b7328ae1).** I ran `babel_update.sh` end to end on this branch, and the `.pot` it produced was rejected by gettext (`msgfmt: missing 'msgstr' section`). The script's last loop deletes the final line of every po/pot file unconditionally. It exists to drop the extra blank line pybabel writes (python-babel/babel#799). Now that msgcat actually runs, the `.pot` reaches that loop straight from msgcat, which writes no trailing blank line. So the delete removed the last entry's `msgstr ""` instead. The loop now deletes the last line only when it is blank. The `.po` files were never affected, because `pybabel update` reads the template before the strip runs. A new end-to-end test runs the real script with real msgcat, and it fails on the old line. **2. `.pot` regenerated with the script itself (a7e85edfcb).** Earlier I normalized the template by running msgcat on it directly. That also dropped the blank line the script puts after the license header. The committed file now matches what `babel_update.sh` produces byte for byte, apart from `POT-Creation-Date`. It has the same 5460 entries as `master`, with no msgid, msgstr, flag or comment changes. **3. Review threads.** Answered inline and resolved: @sadpandajoe's regression-test request (covered by two existing tests) and Bito's ordering-test point (conceded; the test now matches command lines only, in d75bc8da7d). Checked locally on this head: - Translation script tests: 101 pass. - `check_pot_drift.py`: passes. - The `babel-extract` job replayed against base (`check_translation_regression.py`): no count changes in any language. - Re-running the script changes only `POT-Creation-Date`. - `msgfmt --check` accepts the generated `.pot`. - pre-commit passes. - Full `tests/unit_tests`: 11 failures, all date/time tests that fail the same way on `master` locally. It still merges cleanly with #44397, and the combined branch passes the same checks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
