Project

General

Profile

Bug #15362

Feature #10034: Translation web platform

Feature #15079: Integrate the platform with our Git and ikiwiki infrastructure

Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories

Added by u over 1 year ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
03/02/2018
Due date:
% Done:

30%

Feature Branch:
tails.git:translation_platform_hooks
Type of work:
Communicate
Blueprint:
Starter:
Affected tool:

Description

Currently, contribute/l10n_tricks/check_po.sh calls i18inspector and executes some checks. However, it does not check for inconsistent line endings and the like. We should fix that by looking at "git log -p *.po" and determining the manual fixes we regularly do.


Related issues

Related to Tails - Bug #15361: Implement automatic checking & correction of inconsistent strings in Weblate Resolved 03/02/2018
Related to Tails - Bug #15605: Make check_po optionally accept a list of files Resolved 05/17/2018
Related to Tails - Feature #8358: Automatically check PO files in all our repositories Rejected 12/02/2014
Related to Tails - Feature #16102: List of potential checks we might want to do on PO files Resolved 11/05/2018
Blocked by Tails - Feature #16328: Merge a stricter version of check_po whose expectations are realistic into our master branch Resolved 01/08/2019

History

#1 Updated by u over 1 year ago

  • Related to Bug #15361: Implement automatic checking & correction of inconsistent strings in Weblate added

#2 Updated by u over 1 year ago

  • Parent task set to #15079

#3 Updated by u over 1 year ago

  • Subject changed from Improve checks to be done in check_po.sh to Improve integrity checks in check_po.sh

check for missing headers:

- pot-creation-date

check for wrong charsets

- charset should be set and should be utf-8

- inconsistent line endings are currently not checked

#4 Updated by u over 1 year ago

Actually this seems to be checked but as we don't call this regulary, sometimes wrong files get committed.

#5 Updated by u over 1 year ago

  • Subject changed from Improve integrity checks in check_po.sh to Run check_po whenever we try to commit a po file in all Git repositories

We will likely want to add this check to the pre-commit hook:

#!/bin/sh
# If we try to commit po files, check that they do not contain errors.
if ! git diff --ignore-submodules --name-only --exit-code -- . '*.po' \
   && ! ./wiki/src/contribute/l10n_tricks/check_po.sh
then
   echo >&2 "The po files you're trying to commit contain errors. Please fix them and try again." 
fi 

#6 Updated by hefee over 1 year ago

grep -l '"Content-Type: text/plain; charset=UTF-8\n"' *po

should be empty to make sure no file has another charset.

#7 Updated by bertagaz over 1 year ago

  • Target version changed from Tails_3.7 to Tails_3.8

#8 Updated by u over 1 year ago

pre-commit to test:


# If we try to commit po files, check that they do not contain errors.
if test $(git diff --ignore-submodules --name-only --exit-code -- . '*.po' $against |
    LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 ; then
    if ! ./wiki/src/contribute/l10n_tricks/check_po.sh ; then
        echo
        echo "The po files you're trying to commit contain errors. Please fix them and try again." 
        echo
        exit 1
    fi
fi

#9 Updated by u over 1 year ago

  • Feature Branch set to tails.git:translation_platform_hooks

#10 Updated by u over 1 year ago

I've created a branch which shall contain all the hooks and scripts we need to test.

#11 Updated by u over 1 year ago

  • Status changed from Confirmed to In Progress

#12 Updated by u over 1 year ago

  • Related to Bug #15605: Make check_po optionally accept a list of files added

#13 Updated by hefee over 1 year ago

  • % Done changed from 0 to 30

updated the pre-commit hook to process only changed files (ignoring also deleted files).

Side note: The pre-commit hook using the file in the working directy and not the "real" diff that will be commited.
So you can still commit unclean po files if you have outstanding diffs for the po files. I think this is not a bug issue, as you normally will commit what you have changed in further commits. For the main git this is not an issue, as it has no chackedout version.

#14 Updated by u about 1 year ago

hefee wrote:

updated the pre-commit hook to process only changed files (ignoring also deleted files).

Thanks!

So you can still commit unclean po files if you have outstanding diffs for the po files. I think this is not a bug issue, as you normally will commit what you have changed in further commits. For the main git this is not an issue, as it has no chackedout version.

Ack.

#15 Updated by u about 1 year ago

u wrote:

Currently, contribute/l10n_tricks/check_po.sh calls i18inspector and executes some checks. However, it does not check for inconsistent line endings and the like. We should fix that by looking at "git log -p *.po" and determining the manual fixes we regularly do.

The new pre-commit hook checks for whitespace errors.

#16 Updated by intrigeri about 1 year ago

  • Target version changed from Tails_3.8 to Tails_3.9

#17 Updated by u about 1 year ago

  • Related to Feature #8358: Automatically check PO files in all our repositories added

#18 Updated by intrigeri about 1 year ago

What does "in all Git repositories" mean here? Given the parent task of this ticket, I understand it as "all Git repositories for our website", but on #8358 u suggested that it might also apply to our other Git repos (which is what #8358 is about).

#19 Updated by u 12 months ago

intrigeri, thanks for making this clear. I meant only the website Git repositories indeed. This said, it could eventually be applied to others.

#20 Updated by intrigeri 12 months ago

This said, it could eventually be applied to others.

Good to know! I've updated the description of #8358 to list the available options and stated my opinion in a comment.

#21 Updated by u 12 months ago

  • Target version changed from Tails_3.9 to Tails_3.10.1

Won't be able to do this in the next 48 hours (release of 3.9)

#22 Updated by intrigeri 10 months ago

  • Target version changed from Tails_3.10.1 to Tails_3.11

#23 Updated by u 10 months ago

  • Subject changed from Run check_po whenever we try to commit a po file in all Git repositories to Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories
  • Assignee changed from u to hefee

file is here: bin/pre-commit-translation
strategy for implementation unclear.

#24 Updated by emmapeel 10 months ago

I think that the script needs to say more than:

The po files you're trying to commit contain errors. Please fix them and try again.

Like what is the file, what is the problem, etc. Many of our contributors will not know how to get out of that situation.

#25 Updated by u 10 months ago

@emmapeel: Did you test the hook?
Because it calls an updated version of check_po that actually gives you more details on what went wrong.
The error message you quote is simply a last message in the list, when check_po returns an error.

We've sent around requests for testing this hook several months ago, without any reply from translators.
These emails also contained information on how to prepare po files for compliance with check_po.
I can resend them, if needed.

#26 Updated by hefee 10 months ago

  • Related to Feature #16102: List of potential checks we might want to do on PO files added

#27 Updated by hefee 10 months ago

  • QA Check set to Pass

file is here: bin/pre-commit-translation

the file is fine so far. When we rename check_po, than we need also update the filepath.

strategy for implementation unclear.

  1. make pre-commit-hook not failing, just warning:
    replace exit 1 with exit 0
  2. merge the translation_platform_hooks
  3. install the post-update-hook at tails servers, than everyone gets warned about issues in po file
  4. send a mail:
    - in 2 weeks the po files will get updated to pass the new checks.
    - the hook will be activated afterwards to failmode.
    - users should make sure that any non-commited stuff is pushed before, because if we update all po files merge conflicts may happen.
  5. 2 weeks later:
    - update all po files
    - change exit 0 -> exit 1 and make the hook active.
    - send a mail to inform the users.

#28 Updated by CyrilBrulebois 8 months ago

  • Target version changed from Tails_3.11 to Tails_3.12

#29 Updated by intrigeri 7 months ago

  • Blocks Feature #15082: Have the Weblate Git communicate with our main Git repository added

#30 Updated by intrigeri 7 months ago

  • Blocked by Feature #16328: Merge a stricter version of check_po whose expectations are realistic into our master branch added

#31 Updated by intrigeri 7 months ago

  • Blocks deleted (Feature #15082: Have the Weblate Git communicate with our main Git repository)

#32 Updated by intrigeri 7 months ago

Updated plan: don't block on this ticket and for now, focus on #16328.

#33 Updated by anonym 7 months ago

  • Target version changed from Tails_3.12 to Tails_3.13

#35 Updated by CyrilBrulebois 5 months ago

  • Target version changed from Tails_3.13 to Tails_3.14

#36 Updated by intrigeri 4 months ago

  • Target version deleted (Tails_3.14)

intrigeri wrote:

Updated plan: don't block on this ticket and for now, focus on #16328.

Given this + the past history of "Target version" values here, I guess we can drop the target version, at least for now :)

#37 Updated by intrigeri 3 months ago

  • QA Check deleted (Pass)

(Preparing to drop the "QA Check" field as per "[Tails-dev] Proposal: Redmine workflow change". I see no review having happened here.)

#38 Updated by hefee 3 months ago

intrigeri wrote:

(Preparing to drop the "QA Check" field as per "[Tails-dev] Proposal: Redmine workflow change". I see no review having happened here.)

@intrigeri the code was written by u and I reviewd it. That's why I set QA Check as "Pass". So we loose the status, that the code got reviewed, but it is still living in a non merged branch.

#39 Updated by intrigeri about 2 months ago

  • Assignee changed from hefee to intrigeri
  • Target version set to Tails_3.15
  • Type of work changed from Code to Communicate

In the end we did the work to have this pre-commit hook (even if we had decided not to block on it) so I'll announce it.

#40 Updated by intrigeri about 2 months ago

  • Status changed from In Progress to Resolved

#41 Updated by intrigeri about 2 months ago

  • Assignee deleted (intrigeri)

Also available in: Atom PDF