Bug #15605

Make check_po optionally accept a list of files

Added by Anonymous 2018-05-17 14:14:12 . Updated 2019-06-27 17:16:56 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Target version:
Tails_3.15
Start date:
2018-05-17
Due date:
% Done:

100%

Feature Branch:
https://salsa.debian.org/tails-team/tails/merge_requests/14
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Deliverable for:
309

Description

Makes it more useful for users/scripts to test against only modified files and ignore the rest.

Files

0001-Fix-typos-and-minor-improvements.patch (1666 B) Anonymous, 2018-11-02 17:20:26

Subtasks

Related issues

Related to Tails - Bug #15362: Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories Resolved 2018-03-02
Related to Tails - Bug #15408: Consider forcing wrapping of po files at 79 chars per line Rejected 2018-03-13
Related to Tails - Feature #16102: List of potential checks we might want to do on PO files Resolved 2018-11-05
Blocks Tails - Bug #15403: Unify po headers Resolved 2018-08-19

History

#1 Updated by Anonymous 2018-05-17 14:14:30

  • related to Bug #15362: Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories added

#2 Updated by Anonymous 2018-05-17 14:14:45

  • related to Bug #15408: Consider forcing wrapping of po files at 79 chars per line added

#3 Updated by Anonymous 2018-05-17 14:15:10

#4 Updated by hefee 2018-05-17 16:44:39

  • % Done changed from 0 to 60

This is now possible with:
https://github.com/hefee/tails-jenkins-tools/commit/a23231e54185f371b7279d544d51368ca2932826

I needed to fork jenkins-tools as it is a submodule with a own repository.

#5 Updated by Anonymous 2018-05-29 08:26:59

  • Feature Branch set to jenkins-tools.git:translation_platform_hooks

I’ve merged this into our jenkins tools submodule and I guess we will have to ask groente to do another review and merge this into master.

#6 Updated by Anonymous 2018-05-29 08:46:24

  • Assignee changed from hefee to groente
  • QA Check set to Ready for QA

Hi groente, may you please review this?

Our aim here is to be able to use check_po on single files, or a list of files, instead of always running it on the entire repository. This is because we have created a git hook which will run whenever people try to commit po files and then we don’t want to make their life horrible with waiting for check_po…

#7 Updated by groente 2018-06-13 09:12:15

  • Assignee changed from groente to hefee
  • % Done changed from 60 to 80
  • QA Check changed from Ready for QA to Pass

looks good! the only remark i have is on line 112, langauge is misspelled ;)

#8 Updated by Anonymous 2018-06-19 09:04:50

  • Assignee deleted (hefee)

Thanks! I’ll fix this and ask intrigeri to give me his okay to merge this (test suite in mind).

#9 Updated by Anonymous 2018-06-19 09:40:08

  • Assignee set to intrigeri
  • QA Check changed from Pass to Info Needed

The last commit we made should be at 32432bd329d9ba5b5e07e76daf87d7bd8d5ab5ed.
Can you tell me if I need to check something else before merging this? I suppose this script is run in the test suite or somewhere else on the production machine?

#11 Updated by intrigeri 2018-06-19 15:56:28

  • QA Check changed from Info Needed to Ready for QA

> The last commit we made should be at 32432bd329d9ba5b5e07e76daf87d7bd8d5ab5ed.
> Can you tell me if I need to check something else before merging this? I suppose this script is run in the test suite or somewhere else on the production machine?

OK, I’ll take a look (next week).

#12 Updated by intrigeri 2018-06-19 16:03:43

  • Target version set to Tails_3.9

#13 Updated by intrigeri 2018-06-23 07:34:02

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

I suppose it would smooth your work if this gets merged in time for 3.8. So I’ll try to review and hopefully merge this despite the tight timing.

#14 Updated by intrigeri 2018-06-23 07:54:20

I’m a bit confused: the corresponding branch in tails.git updates the jenkins-tools submodule to commit 5332be2098f5564561f6f9eb274b437e97533242, but that’s not the last commit on the branch in jenkins-tools.git (currently: 32432bd329d9ba5b5e07e76daf87d7bd8d5ab5ed). It would be confusing to merge these two branches as is in their respective repos, because then tails.git would reference an old version of jenkins-tools.git. I assume it’s a mere oversight and if happy with the two branches, I’ll bump the submodule in tails.git to its latest version.

#15 Updated by intrigeri 2018-06-23 07:58:20

  • related to deleted (Bug #15403: Unify po headers)

#16 Updated by intrigeri 2018-06-23 07:58:22

#17 Updated by intrigeri 2018-06-23 08:41:49

  • Assignee deleted (intrigeri)
  • QA Check changed from Ready for QA to Dev Needed

I’m fine with the code changes that implement what the title of this ticket says, but the branch has other changes (new checks). I’m not sure where I should comment on these changes, between Bug #15403 and here. Whatever, I’ll do it here.

As part of the release process of our custom packages, we run check_po on PO files that come straight from Transifex. They don’t satisfy all these new requirements. So either the “Upgrade bundled binary Debian packages” section of wiki/src/contribute/release_process.mdwn must be adjusted to use your code that unifies PO headers, or there must be a way to opt-out of these new checks.

Otherwise, code review passes. I’ve pushed a trivial typo fix on top of your branch.

#18 Updated by intrigeri 2018-06-26 16:27:51

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

#19 Updated by Anonymous 2018-08-19 16:24:01

intrigeri wrote:
> I’m fine with the code changes that implement what the title of this ticket says, but the branch has other changes (new checks). I’m not sure where I should comment on these changes, between Bug #15403 and here. Whatever, I’ll do it here.

Ack, thanks.

> As part of the release process of our custom packages, we run check_po on PO files that come straight from Transifex. They don’t satisfy all these new requirements. So either the “Upgrade bundled binary Debian packages” section of wiki/src/contribute/release_process.mdwn must be adjusted to use your code that unifies PO headers, or there must be a way to opt-out of these new checks.

Right. There is currently no way to opt out and we have to think about it. Oversight.

> Otherwise, code review passes. I’ve pushed a trivial typo fix on top of your branch.

Thanks, I’ll have a look.

#20 Updated by Anonymous 2018-09-03 17:54:22

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

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

#21 Updated by intrigeri 2018-10-24 17:03:36

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

#22 Updated by Anonymous 2018-11-02 16:44:07

  • Assignee set to hefee

#23 Updated by Anonymous 2018-11-02 17:18:28

intrigeri wrote:
> Otherwise, code review passes. I’ve pushed a trivial typo fix on top of your branch.

I’m not able update the submodule in the branch (completely able ‘git submodule update —init’ which seems to fetch from whereever, but not the branch I’ve currently checked out) . Any idea how to do this properly?

#24 Updated by Anonymous 2018-11-02 17:20:53

Because I cannot push, I’ll simply add a patch here. Last commit before the patch in this branch was 32432bd329d9ba5b5e07e76daf87d7bd8d5ab5ed

#25 Updated by intrigeri 2018-11-05 09:15:14

(I’m not sure if you’re asking me. In doubt, happy to help with submodules stuff :)

I don’t understand what “completely able ‘git submodule update —init’ which seems to fetch from whereever, but not the branch I’ve currently checked out” means.

FWIW, I can fetch the submodule just fine and the state of the branch in jenkins-tools.git seems to be as intended:

$ cd submodules/jenkins-tools/ && git fetch
$ git remote -v
origin  tails@git.tails.boum.org:jenkins-tools (fetch)
origin  tails@git.tails.boum.org:jenkins-tools (push)
$ git log -1 --pretty=oneline origin/translation_platform_hooks
44520f80b8dce9657dfeaae36650a3725ce82d1c (origin/translation_platform_hooks) Fix typo.

Does this produce a different output for you?

Now, the branch in tails.git references an old version of the submodule:

$ cd /my/tails/git/checkout && git checkout translation_platform_hooks && git reset --hard @{u} && git submodule update --init
$ cd submodules/jenkins-tools
$ git log --pretty=oneline ..origin/translation_platform_hooks
44520f80b8dce9657dfeaae36650a3725ce82d1c (origin/translation_platform_hooks) Fix typo.
32432bd329d9ba5b5e07e76daf87d7bd8d5ab5ed Merge remote-tracking branch 'hefee/translation_platform_hooks' into translation_platform_hooks
2fc2faa5c5335046cb3162eddd03819d3b8634e0 check for 79 chars.
a9e1565c0a752541af3fc2b3b71b624c4c9fb3e1 Minor spacing or comment improvements

So presumably you need to do something like this:

$ cd /my/tails/git/checkout && git checkout translation_platform_hooks && git pull && git submodule update --init
$ cd submodules/jenkins-tools
$ git checkout translation_platform_hooks
$ git merge origin/translation_platform_hooks
# here you apply your additional commit
$ git push -u origin translation_platform_hooks
$ cd ../../
$ git commit -m "Update the jenkins-tools submodule." submodules/jenkins-tools
$ git push origin translation_platform_hooks

Happy to help if that’s not enough, once I understand what exact problem you’re facing.

#26 Updated by hefee 2018-11-05 16:02:59

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

#27 Updated by hefee 2018-11-05 16:05:54

  • Status changed from In Progress to Fix committed
  • % Done changed from 80 to 100

The original bug “accept a list of files” for check_po is solved.
About @u issue of not able to push - please move this discussion to another place.

#28 Updated by CyrilBrulebois 2018-12-16 13:21:57

  • Status changed from Fix committed to Resolved

#29 Updated by hefee 2018-12-17 19:42:29

  • Status changed from Resolved to In Progress
  • Target version changed from Tails_3.11 to Tails_3.12
  • QA Check changed from Dev Needed to Ready for QA

I used Fix commited as status, as it is commit on a local branch, that will together with other task review together. I use now the more general “In Progress” for the moment.

#30 Updated by intrigeri 2018-12-18 08:20:30

> I used Fix commited as status, as it is commit on a local branch, that will together with other task review together.

In general “Fix committed” means “reviewed and merged somewhere that will for sure be released/deployed”. So “In progress” seems indeed closer to what you mean :)

#31 Updated by anonym 2019-01-30 11:59:14

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

#32 Updated by Anonymous 2019-02-07 15:27:39

#33 Updated by hefee 2019-02-08 23:02:04

  • Description updated
  • Status changed from In Progress to Resolved

#34 Updated by hefee 2019-02-08 23:03:02

  • Subject changed from Make check_po.sh optionally accept a list of files to Make check_po optionally accept a list of files

Maked it as resolved, as the new check_po has this feature implemented (not reviewed yet).

#35 Updated by hefee 2019-02-08 23:03:16

  • Status changed from Resolved to In Progress

#36 Updated by Anonymous 2019-02-13 11:48:28

I see you reopened it: good. “Resolved” means: reviewed & merged.
I’m not sure who will review this one. I would propose that this be assigned to intrigeri for review and merging.

#37 Updated by intrigeri 2019-03-12 17:10:36

  • Assignee changed from hefee to intrigeri
  • Target version changed from Tails_3.13 to Tails_3.15

Will review this at the sprint.

#38 Updated by intrigeri 2019-06-01 14:29:53

  • Assignee changed from intrigeri to hefee
  • QA Check changed from Ready for QA to Info Needed
  • Feature Branch deleted (jenkins-tools.git:translation_platform_hooks)

@hefee, could you please point the “Feature Branch” field to wherever the code I should review lives? (jenkins-tools.git:translation_platform_hooks was last modified almost a year ago, and does much more than what this ticket is about, so I assume that’s not it.)

#39 Updated by intrigeri 2019-06-02 15:19:40

  • QA Check deleted (Info Needed)

#40 Updated by hefee 2019-06-03 09:38:54

  • Assignee changed from hefee to intrigeri
  • Feature Branch set to https://salsa.debian.org/tails-team/tails/merge_requests/14

As the complete check_po got rewritten, this is now done with unify_po-headers.py (see Bug #15403).

#41 Updated by intrigeri 2019-06-07 10:36:00

  • Status changed from In Progress to Needs Validation

Thanks!

#42 Updated by intrigeri 2019-06-22 15:36:20

  • Status changed from Needs Validation to In Progress
  • Assignee changed from intrigeri to hefee

hefee wrote:
> As the complete check_po got rewritten, this is now done with unify_po-headers.py (see Bug #15403).

@hefee, then I don’t understand what I’m supposed to review this (Bug #15605) very ticket. I see there’s an ongoing review process for !14 by enrico and I’d rather not start another concurrently one. Could you please clarify what you’re expecting from me here? Thanks in advance!

#43 Updated by hefee 2019-06-24 09:04:08

intrigeri wrote:
> @hefee, then I don’t understand what I’m supposed to review this (Bug #15605) very ticket. I see there’s an ongoing review process for !14 by enrico and I’d rather not start another concurrently one. Could you please clarify what you’re expecting from me here? Thanks in advance!

enrico reviews only the security impact of the script and not the features. So your review step would be to check if the new script is able to handle a list of files. But yeah you can and should comment on !14 and not start a second one.

#44 Updated by intrigeri 2019-06-24 09:07:18

  • Status changed from In Progress to Needs Validation
  • Assignee changed from hefee to intrigeri

> So your review step would be to check if the new script is able to handle a list of files. But yeah you can and should comment on !14 and not start a second one.

Thanks! :)

#45 Updated by intrigeri 2019-06-24 12:05:14

  • Status changed from Needs Validation to In Progress
  • Assignee changed from intrigeri to hefee

See the Salsa MR :)

#46 Updated by intrigeri 2019-06-25 13:13:46

  • Status changed from In Progress to Resolved

#47 Updated by intrigeri 2019-06-27 17:16:56

  • Assignee deleted (hefee)