Skip to content
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

Multi onesky projects for translations #4

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

BourotBenjamin
Copy link

Hi,

I updated this project because we some specific needs.
We have one Symphony project, and we need to send translations in more than one OneSky's projects.
So here is the solution I provide :
I updated the config like that :

file_paths:
    123:
        - "%kernel.root_dir%/Resources/translations"
    456:
        - "%kernel.root_dir%/Resources/entities-translations"

With this modifications, we set a project id for one (or more) paths and it upload files directly on good projects.

In addition, I add to the commands a optional "projectId" for the commands (which is needed when you set filePath option ) :
If you set only filePath, as he don't know his project id, he pull or push every files from every projects
If you only set projectId, he pulls (or push) every files from (to) the wanted project.
If you set filePath and projectId, he pulls (or push) files to (from) folders given from (to) the wanted project.

@coveralls
Copy link

coveralls commented Nov 15, 2016

Coverage Status

Coverage decreased (-14.9%) to 85.132% when pulling 8ce41e1 on BourotBenjamin:master into 3b33b03 on OpenClassrooms:master.

@romainkuzniak
Copy link
Contributor

Thank you for this PR!

I think a better way to handle the different projects should be to enable an array of project in the config and default settings.
This way, each project could override the default settings

So the config file should look like this:

# app/config/config.yml

openclassrooms_onesky:
    api_key:  %onesky.api_key%
    api_secret: %onesky.api_secret%
    source_locale: %source_locale% #optional, default en
    locales:
        - fr
        - es
    file_format: %onesky.file_format% #optional, default xliff
    keep_all_strings: false # default true
    projects:
        projectName:
            id: %onesky.project_id%
            source_locale: %source_locale% #optional, default main setting
            locales: #optional, default main setting
                - fr
                - es
            file_format: %onesky.file_format% #optional, default main setting
            file_paths:
                - %path.to.translations.files.directory%
            keep_all_strings: true #optional, default main setting

You have to know that file path could be wild card. I’m going to update the current documentation.
Another thing, you PR broke the build, and decrease the cover. Could you please fix it?

Thank you

@BourotBenjamin
Copy link
Author

I saw, I would try to fix it tomorrow. ;)

@coveralls
Copy link

coveralls commented Nov 16, 2016

Coverage Status

Coverage decreased (-0.5%) to 99.523% when pulling 8f146bb on BourotBenjamin:master into 3b33b03 on OpenClassrooms:master.

@BourotBenjamin
Copy link
Author

I fixed the tests to work with the new config, I also had to change the guzzle exception as this exception doesn't' event appears on Guzzle git repo.
It seems to be an Amazon AWS exclusive exception.
So I changed it for a Symphony's HTTP Exception with status code = 500.

@coveralls
Copy link

coveralls commented Mar 30, 2017

Coverage Status

Coverage decreased (-5.4%) to 94.575% when pulling c87e394 on BourotBenjamin:master into 3b33b03 on OpenClassrooms:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.9%) to 97.09% when pulling 495c199 on BourotBenjamin:master into 3b33b03 on OpenClassrooms:master.

@BourotBenjamin
Copy link
Author

Hi,

I updated this PR but I didn't manage to find how to handle two different travis configurations for the tests.
As php 7 needs yaml-2.0.0 and php 5.* needs yaml pecl extensions, php 5.* build would failed so I removed it from the travis configuration.
But if you new how to handle this case, I would be happy to know. :)

Thanks.

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

Successfully merging this pull request may close these issues.

3 participants