Skip to content

#813 #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 26, 2023
Merged

#813 #814

merged 18 commits into from
Apr 26, 2023

Conversation

ml4nC3
Copy link
Contributor

@ml4nC3 ml4nC3 commented Apr 20, 2023

Please check if the PR fulfills these requirements:

  • The commit message follows our guidelines.
  • For bug fixes and features:
    • You tested the changes.

Related issue (if applicable): #813

What kind of change does this PR introduce?
Introducing a script to integrate translations from GD-Learn-Translations if their completion is good enough.

Does this PR introduce a breaking change?
I don't think so...

New feature or change

What is the current behavior?
extract.py script and sync-translations.py script have to be executed manually. Once done it is difficult to know how well a translation is up to date regarding newly generated POT files.

What is the new behavior?
The additional script check_and_integrate_translations.py execute sequentially extraction and sync, then looks for untranslated and fuzzy entries in PO files in order to compute some completion indicator.

If a threshold value is given it then copy translations back to GD-Learn if their completion is above.

Other information
! beware this pull request includes a revert on commit ml4nC3@297bd15

This commit seems to be responsible for msgid mismatch issues in the application when PO files are integrated back.

@NathanLovato
Copy link
Contributor

NathanLovato commented Apr 26, 2023

Thanks for the work. Got a little refactoring to do, mainly in the future I'd recommend using Python's argparse module if you haven't as it's less error-prone than the old getopt.

For example, this code will handle the help message + validate every argument properly and return you a neat dictionary. You don't have to keep the help, argument logic, and testing arguments separate.

def parse_command_line_arguments():
    """
    Parses command line arguments and returns a dictionary containing options and arguments.
    """
    parser = argparse.ArgumentParser(description="This script performs text extraction from the application and move "
                                                 "generated POT files in the translations project in order to compare "
                                                 "with translations source and output a translation completion "
                                                 "indicator for each language.")
    parser.add_argument("translations_path", help="relative or absolute path to GD-Learn-translations directory.")
    parser.add_argument("-t", "--threshold", type=int, help="Minimum completion percentage value for a language to be integrated. "
                                                            "Language integration is skipped if no value is given.")
    parser.add_argument("-E", "--skip-extract", action="store_true", help="Skip the extraction of strings and POT files generation.")
    parser.add_argument("-S", "--skip-sync", action="store_true", help="Skip the synchronization and merge of PO files with the reference POT files.")
    return vars(parser.parse_args())

The dictionary returned by the function looks like this:

{'translations_path': 'test', 'threshold': None, 'skip_extract': False, 'skip_sync': False}

Anyway I'll refactor, test, and this should be good to merge.

@NathanLovato
Copy link
Contributor

Just tackled the first half, I pushed the changes to your branch directly. I had only a good half hour to work on this but will refactor the second half and merge ASAP, hopefully tonight, otherwise, tomorrow.

@NathanLovato
Copy link
Contributor

Note that I'm pushing commits directly to your branch, so you don't have anything to do, as mentioned before this makes the contribution process more efficient for everyone. If you have any questions about the changes don't hesitate to use the Files changed tab to leave comments.

@NathanLovato
Copy link
Contributor

Another note regarding additions to .gitignore: in general if you participate to open source projects maintainers will ask you not to push changes that are specific to your development environment.

Instead of using the shared .gitignore file, you can add exclude patterns to .git/info/exclude, which is always local to your copy of the repository.

@NathanLovato NathanLovato merged commit 697c488 into GDQuest:main Apr 26, 2023
@NathanLovato
Copy link
Contributor

NathanLovato commented Apr 26, 2023

Thank you so much for taking the time to work on this! I'll take it for a spin and make a new release by the end of the week :)

I think you said you're French too, right? If you happen to go to Game Camp in June, in Lille, ( https://gamecamp.fr/ ) let me know! My teammate and I will be there

@ml4nC3
Copy link
Contributor Author

ml4nC3 commented Apr 27, 2023

Got a little refactoring to do, mainly in the future I'd recommend using Python's argparse module if you haven't as it's less error-prone than the old getopt.

Also good to know. I have to admit I saw both solutions in documentation and choose getopt for its simplicity, but the result with arparse is indeed much cleaner.

Thank you so much for taking the time to work on this! I'll take it for a spin and make a new release by the end of the week :)

You're welcome, I'm glad I helped ! Don't forget there is still this strange behavior regarding the wrapped strings that resulted in unrecognized translations in the end.

The script will do it's job, but we might not be quite at the end of the road yet. Let me know how it went with your tests. Maybe you can open another issue if you spot something ? You can get my screenshot and comments if needed.

I've scrolled into GD-Learning code without much success on spotting where strings are actually used. If you need me to continue I might need a few advices on which scenes or GD scripts to look into.

I think you said you're French too, right? If you happen to go to Game Camp in June, in Lille, ( https://gamecamp.fr/ ) let me know! My teammate and I will be there

Yes ! I live in Nantes. I'd love to go and meet you there ! Unfortunately I'm pretty sure I won't make it, I have already several work appointments on those dates.

But don't worry, I'll stick around ! This first contribution was a really good experience to me. Helps me keeping a few fingers in true development work. You're refactor is also very interesting. So I'll see if there is another issue that I can give a hand on. Feel free to send suggestions if you have anything in mind, now or later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automated Build System to Check and Integrate Language Translations
2 participants