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

Add CI step for checking if translations are properly marked as outdated #6428

Merged
merged 3 commits into from
Nov 29, 2021

Conversation

TicClick
Copy link
Contributor

@TicClick TicClick commented Nov 23, 2021

what

this check protects us from the situations when someone has edited an original article (en.md), but forgot to check its translations and mark them as outdated by adding outdated: true and outdated_since: {commit hash} markers to their front matters.

for cases which can't be handled automatically (say, you're fixing a typo which is only present in English article), it can be silenced by adding SKIP_OUTDATED_CHECK to the pull request's message (it may appear everywhere).

example: TicClick#2. see CI output on individual commits, as well as its status for the whole PR (forced to OK by the tag):

where

it's... uh, inlined in the continuous-integration.yml file. could be moved into a separate script if necessary. if deemed useful, could be sped up by throwing out unnecesary steps like linters. could be moved to a separate workflow and sped up by caching the repository checked out.

when

The script is triggered on default pull_request-related events plus PR message edits to catch the (dis)appearance of SKIP_OUTDATED_CHECK.

why

...the check?

part of #6233. prevents articles expiring silently because of random maintainers' occasional lazy eye syndrom (at least #6197, #6189, #6111, #4651 -- found by me only).

...is this a GitHub Action?

  • needs to be run on every commit
  • should block merge
  • may be overriden
  • the source is easily accessible and editable
  • I honestly don't want to host a bot, or an app, not to mention that it'd have been a bit more complicated

...do you think people will follow it?

it prints errors AND direct suggestions on how to fix them. besides, it's important to know which translations are really up to date.

add CI step for checking if translations are marked as outdated
@TicClick TicClick added the area:meta non-article files label Nov 23, 2021
@peppy
Copy link
Member

peppy commented Nov 23, 2021

The error message should probably better explain where the SKIP_OUTDATED should be added. It's a bit unclear to users that first see it in that message.

@TicClick TicClick mentioned this pull request Nov 23, 2021
7 tasks
@The-Last-Cookie
Copy link
Contributor

Might it be a good idea to put a small comment containing SKIP_OUTDATED into the PR template?
I don't want to spend extra time adding this tag in the PR message after the PR has been opened.

@TicClick
Copy link
Contributor Author

I am against this. the situations where you need to willingly ignore the check are much more rare compared to a missing outdated tag here and there — you only need this knowledge occasionally and not in advance. the description will also be much longer than the rest of the points from the PR template comment, partly because what's happening is hard to describe in a few words, and will lenghten it even more, which will lead to less people reading the template as a whole at all

@peppy
Copy link
Member

peppy commented Nov 29, 2021

Let's give this a try and see how it works. The script looks sane enough.

@peppy peppy merged commit 3488157 into ppy:master Nov 29, 2021
@TicClick TicClick deleted the ci-outdated-translations branch November 29, 2021 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:meta non-article files size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants