Skip to content

Add padpo to travis-ci.#1059

Merged
JulienPalard merged 4 commits into
python:3.8from
JulienPalard:padpo
Dec 12, 2019
Merged

Add padpo to travis-ci.#1059
JulienPalard merged 4 commits into
python:3.8from
JulienPalard:padpo

Conversation

@JulienPalard

@JulienPalard JulienPalard commented Dec 12, 2019

Copy link
Copy Markdown
Member

For the moment I keep a || true so it can never fail, just print the report.

Closes #1057

This implements @vpoulailleau idea of whitelisting already known padpo errors so we just never introduce new ones.

vpoulailleau
vpoulailleau reviewed Dec 12, 2019
View reviewed changes

@vpoulailleau vpoulailleau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree for the || true as a starting point, though a message given by padpo is either something to be corrected in the translation or in padpo.

The content of padpo.ignore looks strange to me:

  • sometimes there is a [ in the line
  • I would not have taken the line numbers into account, because they can be changed easily (powrap for instance), I would have kept the error messages

For instance, for the following padpo message:

2019-12-12 11:03:05,573 [ERROR   ]            pofile.py(153):display_warnings     :: /tmp/padpo_e_1dtjy0/whatsnew/3.3.po:  21 [ Line length  ] Line too long (90 > 79): Cet article présente les nouvelles fonctionnalités de Python 3.3, comparé à Python 3.2.

I would have kept:

[ Line length  ] Line too long (90 > 79): Cet article présente les nouvelles fonctionnalités de Python 3.3, comparé à Python 3.2.

What do you think?

@JulienPalard

JulienPalard commented Dec 12, 2019

Copy link
Copy Markdown
Member Author

@vpoulailleau is it better?

@vpoulailleau

vpoulailleau commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

@vpoulailleau is it better?

@JulienPalard Yes. I'm wondering about grep results with special characters at the end of the messages. But let's see in CI, since we have the || true.

@JulienPalard

JulienPalard commented Dec 12, 2019

Copy link
Copy Markdown
Member Author

@JulienPalard Yes. I'm wondering about grep results with special characters at the end of the messages.

I used -F so I don't fear that a lot.

@vpoulailleau

vpoulailleau commented Dec 12, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

We also have to try with a failing translation (edit: it has a failing translation… or a padpo bug… 😉).

BTW you made me discover a bug in padpo. padpo.ignore is too large a file, and therefore has no diff in GitHub API response… vpoulailleau/padpo#12

@vpoulailleau

vpoulailleau commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

The CI says:

$ if [ -n "$CHANGED_FILES" ]; then padpo -i "$CHANGED_FILES" 2>&1 | grep -v -Ff padpo.ignore || true; fi

2019-12-12 11:20:06,155 [WARNING ]       grammalecte.py(161):install_grammalecte  :: Missing grammalecte, trying to install it

Processing /tmp/padpo_grammalecte_knh2u6i1/Grammalecte-fr-v1.5.0

Building wheels for collected packages: Grammalecte-fr

  Building wheel for Grammalecte-fr (setup.py): started

  Building wheel for Grammalecte-fr (setup.py): finished with status 'done'

  Created wheel for Grammalecte-fr: filename=Grammalecte_fr-1.5.0-cp37-none-any.whl size=3241981 sha256=16da67286368c0517a947744534a2b0aea42b204fe410d17dec9ed0ee646f4a0

  Stored in directory: /home/travis/.cache/pip/wheels/b5/1f/5e/221e4f57f77a47c94b5d8fbeaa92f37ab57fe923a89560c78b

Successfully built Grammalecte-fr

Installing collected packages: Grammalecte-fr

Successfully installed Grammalecte-fr-1.5.0

2019-12-12 11:20:14,289 [ERROR   ]            pofile.py(153):display_warnings     :: reference/expressions.po:2148 [     NBSP     ] There should be a non-breakable space before ":": between ###emented » sont des singletons. ### and ###:PEP:`8` conseille de toujours ###

The command "if [ -n "$CHANGED_FILES" ]; then padpo -i "$CHANGED_FILES" 2>&1 | grep -v -Ff padpo.ignore || true; fi" exited with 0.

Maybe we can add a grep -v "Grammalecte-fr"? And a grep -v "install_grammalecte"?

@JulienPalard

JulienPalard commented Dec 12, 2019

Copy link
Copy Markdown
Member Author

@vpoulailleau Hey a false positive for you:

2019-12-12 11:20:14,289 [ERROR   ]            pofile.py(153):display_warnings     :: reference/expressions.po:2148 [     NBSP     ] There should be a non-breakable space before ":": between ###emented » sont des singletons. ### and ###:PEP:`8` conseille de toujours ###

@vpoulailleau

vpoulailleau commented Dec 12, 2019

Copy link
Copy Markdown
Contributor

@vpoulailleau Hey a false positive for you:

I edited my previous comment because I saw that 😉.

I put this false positive so that you can test you message filtering, or maybe not 😛

@vpoulailleau vpoulailleau mentioned this pull request Dec 12, 2019
Closed
@JulienPalard JulienPalard merged commit f1679d9 into python:3.8 Dec 12, 2019
@JulienPalard JulienPalard deleted the padpo branch June 7, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Seluj78 Seluj78 Seluj78 approved these changes
@awecx awecx Awaiting requested review from awecx
+1 more reviewer
@vpoulailleau vpoulailleau vpoulailleau left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add padpo to our CI

3 participants

@JulienPalard @vpoulailleau @Seluj78