Safe mode, remove 3.0/3.1 code, unit test refactoring, Symfony upgrade - Githubissues

phpbb / phpbb-translation-validator

A tool checking packages for compliance with the policies https://area51.phpbb.com/docs/dev/3.2.x/language/guidelines.html

GNU General Public License v2.0

8 stars 8 forks source link

Safe mode, remove 3.0/3.1 code, unit test refactoring, Symfony upgrade #54

Closed battye closed 5 years ago

battye commented 5 years ago

List of changes:

Safe mode - allow validation to be performed using file parsing rather than includes if the --safe-mode option is specified.
3.0/3.1 code references removed
Refactored unit tests to go in the tests/ directory
- Tests run correctly again now
Upgraded Symfony components to use v3

battye commented 5 years ago

Travis CI build works again now: https://travis-ci.org/phpbb/phpbb-translation-validator/builds/482318734?utm_source=github_status&utm_medium=notification

So just a quick overview for anyone interested in looking at the code in two pieces:

https://github.com/phpbb/phpbb-translation-validator/pull/53 - contains changes for just the safe mode option
https://github.com/battye/phpbb-translation-validator/pull/1 - contains the the 3.0 and 3.1 refactoring changes/unit test changes (on top of the safe mode changes)

This pull request (#54) contains all of the changes combined. If all code reviews and manual tests are okay, this will be the code that ends up in master and the PR's listed above will just be closed.

battye commented 5 years ago

I've upgraded the Symfony components to v3 because I started integrating it with Titania and Composer was complaining about a few conflicts. (so for anyone testing this don't forget to run composer update)

battye commented 5 years ago

Note to myself:

Make this field nullable in Titania: https://github.com/battye/customisation-db/blob/e56d52720165a559fc492dd25e65ac222673cbf4/migrations/translation_validation_field.php#L37
Remove the temporary change I made in my own database and re-test

battye commented 5 years ago

I think the plan from here, if @Crizz0 agrees is:

Test the changes in this PR: a) Run the current version (v1.4.1) ofphpbb-translation-validator over a whole bunch of different languages and record the output. b) Run the same languages over the version in this pull request in both safe mode and regular mode. Do the results match the v1.4.1 output? If so that suggests it is working. It's expected that in safe mode there will be more notices though.
Fix any issues and make any necessary changes.
When it's reviewed, tested and approved - merge into master a) Release phpbb-translation-validator as v1.5.0
Update the composer.json inside https://github.com/battye/customisation-db/tree/feature/ptv-integration to point to the new v1.5.0 version here a) Submit that as a PR to the Titania repository

DavidIQ commented 5 years ago

@Crizz0 any additional comments here before continuing?

Crizz0 commented 5 years ago

@1.: 👍 @2.: 👍 @3.: yes, maybe some new folder for the files and remove of legacy code/files before 1.5.0, because battye and me discovered that the structure from the <=1.4.x .zip has many things, you don't need at all. So this repo could be made downloadable for everyone @4.: 👍

DavidIQ commented 5 years ago

This is targetting the develop branch. Should it be targeting master instead?

Crizz0 commented 5 years ago

No, master is for functional-versions, we use develop to test and improve those changes.