Skip to content

Add a translation validation tool.#1653

Merged
Frenzie merged 1 commit intoFreshRSS:devfrom
aledeg:improve-travis
Oct 1, 2017
Merged

Add a translation validation tool.#1653
Frenzie merged 1 commit intoFreshRSS:devfrom
aledeg:improve-travis

Conversation

@aledeg
Copy link
Copy Markdown
Member

@aledeg aledeg commented Oct 1, 2017

It's triggered by Travis to check what is missing.

@Alkarex Alkarex added the I18n 🌍 Translations label Oct 1, 2017
@Alkarex Alkarex added this to the 1.9.0 milestone Oct 1, 2017
phpcs . --standard=phpcs.xml --warning-severity=0 --extensions=php -p
fi
- |
if [[ $CHECK_TRANSLATION == yes ]]; then
Copy link
Copy Markdown
Member

@Frenzie Frenzie Oct 1, 2017

Choose a reason for hiding this comment

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

Awesome work on the PHP side, but I have a little nitpick on the Travis/shellscript side. I think it makes more sense to just split it up in a bunch of true vars. :-)

env:
  matrix:
    - CHECK_TRANSLATION=1 # or true/yes/whatever
    - CODE_SNIFFER=1
    - PHP_UNIT=1
    - CHECK_TRANSLATION=1 PHP_UNIT=1 # look, a combo!

Then the script is simply something like:

script:
  - if [ "$CHECK_TRANSLATION" = 1 ]; then php tools/check.translation.php -r; fi
  - if [ "$CODE_SNIFFER" = 1 ]; then phpcs . --standard=phpcs.xml --warning-severity=0 --extensions=php -p; fi

If more complicated scripts are desired in the future you can adapt it like:

  - if [ "$SOMETHING" = something ]; then source .ci/something_whatever.sh; fi # or without source; but don't forget some kind of source common.sh in each script in that case

Implicitly, I also object to Bashisms like [[ $bla == lala ]] unless they bring something unique to the table. :-P

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've done the source thing in the past but it can become a mess. I prefer having everything in the same file.

Copy link
Copy Markdown
Member

@Frenzie Frenzie Oct 1, 2017

Choose a reason for hiding this comment

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

I disagree as I think .travis.yml is horribly unsuited for evaluating the correctness of scripts. Even for simple one liners it's awkward, but that's neither here nor there. My example was poorly chosen because it suggests multiple files even though for our purposes a single script.sh would be much better for the foreseeable future.

What I'm saying is that I think explicit enabling of checks is more elegant than explicit everything.

PS
A .ci/script.sh would look like this:

#!/bin/bash
if [ "$CHECK_TRANSLATION" = 1 ] then
  php tools/check.translation.php -r
fi
if [ "$CODE_SNIFFER" = 1 ] then
  phpcs . --standard=phpcs.xml --warning-severity=0 --extensions=php -p
fi

$ignore['fr']['sub.php'][] = 'feed.description';
$ignore['fr']['sub.php'][] = 'feed.number_entries';


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Stray newline. (I really am nitpicking today, aren't I. 👼 )

@aledeg
Copy link
Copy Markdown
Member Author

aledeg commented Oct 1, 2017

I've updated the code

It's triggered by Travis to check what is missing.
@Frenzie
Copy link
Copy Markdown
Member

Frenzie commented Oct 1, 2017

Alright, cool!

@Frenzie Frenzie merged commit 51e69e9 into FreshRSS:dev Oct 1, 2017
Alkarex added a commit that referenced this pull request Oct 1, 2017
@aledeg aledeg deleted the improve-travis branch October 1, 2017 20:05
@Alkarex Alkarex modified the milestones: 1.9.0, 1.8.1 Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I18n 🌍 Translations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants