Skip to content

ci: add per-PR EN-Revision hash check#413

Closed
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:feat/check-en-revision
Closed

ci: add per-PR EN-Revision hash check#413
lacatoire wants to merge 1 commit into
php:masterfrom
lacatoire:feat/check-en-revision

Conversation

@lacatoire

Copy link
Copy Markdown
Member

Adds a GitHub Actions workflow that runs on every pull request and verifies that the EN-Revision header in modified XML files points to the latest doc-en commit for that file.

How it works

  • Triggered on pull_request events targeting master
  • Collects the list of .xml files changed in the PR
  • For each file that also exists in doc-en, reads the declared EN-Revision hash
  • Compares it against git log -1 on the matching doc-en file
  • Emits a ::error annotation and fails if the hash is missing or outdated

This ensures that translation updates always reference the exact upstream commit they are based on, making future sync work easier to track.

Prior art

This check is already in use in doc-fr, doc-ru, and doc-es. There is also an ongoing initiative in doc-base to provide reusable workflows that all doc-* repos can share.

@KentarouTakeda

KentarouTakeda commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this, and for the work across the other translations. I've measured it against doc-ja's master and I'm going to decline it.

git log -1 returns the latest commit touching the file, [skip-revcheck] included. doc-base defines "in sync" as a set rather than a single hash: RevcheckFileItem::addGitLogData() collects the topmost run of [skip-revcheck] commits plus the first normal commit into hashList, and isSyncHash() accepts any of them. This check accepts only hashLast.

On ja 65877f15 against doc-en 3c48a7f9: revcheck.php ja reports 52 files outdated, this check fails 580, and for 523 of those every intervening doc-en commit is [skip-revcheck].

That gap is a defect doc-base already documents. scripts/translation/lib/backport.php works through consecutive [skip-revcheck] commits and names 4d17[skip-revcheck] Convert class markup to be compatible with DocBook 5.2 — as one that "will mark all its files as outdated" when it should not. php/doc-base#181, open since 2024-11-18, is the proposed fix and the root cause of php/doc-base#133.

4d17 is also what would have blocked four files in #400. One of them, reference/image/gdfont.xml, is identical to current doc-en on the element in question and up to date per revcheck; this check fails it.

Timing matters too. alfsb has just validated the consecutive-[skip-revcheck] fix on php/doc-en#5699 ("Tested on all languages. No breakages, no mass marking files as outdated on revcheck"), which makes a mass <para><simpara> conversion practical. If that lands, git log -1 is rewritten for every file it touches — roughly 5,089 of doc-ja's 8,050 files would go red while remaining correct per revcheck.

Separately, the check evaluates against doc-en HEAD at run time, so a green PR turns red when doc-en touches one of its files, with nothing the contributor can do about it.

Closing. Happy to revisit once php/doc-base#181 or its successor lands and the check can consult revcheck's hash list rather than git log -1. For context, doc-ja leaves EN-Revision unchanged across [skip-revcheck] commits as doc-ru does, while doc-fr and doc-es bump to the latest hash — which is why it behaves differently here.

@alfsb

alfsb commented Jul 24, 2026

Copy link
Copy Markdown
Member

@lacatoire

"The hash that mark a file as ok" seems simple at first, but is really complicated. One possible solution would be revcheck lib exporting the per file "ok list" of hashes, so this test would only be possible after running revcheck itself. But please, do not try to make changes here, as this is.. a very delicate place of doc-base.

You may perhaps create a companion of genrevdb.php, something like a genrevhashes.php, that dumps the per file "ok hashes" somewhere in doc-base/temp, so a test like this could be used in more translations.

@KentarouTakeda

I sincerely thought no one understands these code parts, or the reasons as they need to be so complicated, to cover many different styles of file tracking between translation teams.

Thanks, your reasoning above made my day a lot happier.

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.

3 participants