Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Translation] Added Crowdin Translation Provider#40947

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.xfromandrii-bodnar:feature/crowdin-provider
May 9, 2021
Merged

[Translation] Added Crowdin Translation Provider#40947

fabpot merged 1 commit intosymfony:5.xfromandrii-bodnar:feature/crowdin-provider
May 9, 2021

Conversation

@andrii-bodnar
Copy link
Contributor

@andrii-bodnarandrii-bodnar commentedApr 25, 2021•
edited
Loading

QA
Branch?5.x
Bug fix?no
New feature?no
Deprecations?no
Tickets
LicenseMIT
Doc PR

To follow up on#38475, this PR addsCrowdin Provider.
This provider was removed a few weeks ago from the Translation Providers feature by@welcoMattic.

We discussed all the recent changes made onProviderInterface,TranslatorBagInterface, and others and I already applied these changes to Crowdin Provider.

Also, this Provider is adapted to work with bothCrowdin andCrowdin Enterprise.

The todo list to make it ready is:

  • Write integration tests by mocking HTTP Responses

I will make it done before the beginning of May.

Koc and welcoMattic reacted with thumbs up emojidies reacted with heart emoji
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 5.x branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Thank you for working on this 👍

@andrii-bodnar
Copy link
ContributorAuthor

@OskarStark thanks for the review 👍

OskarStark reacted with rocket emoji

@andrii-bodnarandrii-bodnar changed the title[Translation] Added Crowdin Provider[Translation] Added Crowdin Translation ProviderApr 26, 2021
@andrii-bodnar
Copy link
ContributorAuthor

It is ready for me for a review 🙂
cc@welcoMattic@OskarStark@Nyholm@nicolas-grekas

@NyholmNyholm added this to the5.3 milestoneApr 28, 2021
@andrii-bodnar
Copy link
ContributorAuthor

@stof thanks for the review 👍

@andrii-bodnar
Copy link
ContributorAuthor

@OskarStark@stof, all comments have been addressed except the one related to the concurrency features ofsymfony/http-client. I need to investigate this issue in more detail 🙂

@andrii-bodnar
Copy link
ContributorAuthor

@stof@welcoMattic I've addressed all the comments. It looks ready for another review.

welcoMattic reacted with thumbs up emoji

Copy link
Member

@welcoMatticwelcoMattic left a comment

Choose a reason for hiding this comment

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

Some important comments about loops not being necessary, but this is near to look good to me :+1 :

@andrii-bodnar
Copy link
ContributorAuthor

@welcoMattic I've addressed all your recent comments.

Replied and not resolved two of them - please take a look and resolve if ok 🙂

P.S. Failed appveyor build seems unrelated to my changes.

welcoMattic reacted with thumbs up emoji

@fabpot
Copy link
Member

Thank you@andrii-bodnar.

andrii-bodnar reacted with heart emoji

@fabpotfabpot merged commit6bfb509 intosymfony:5.xMay 9, 2021
@fabpotfabpot mentioned this pull requestMay 9, 2021
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMay 19, 2021
This PR was submitted for the 5.4 branch but it was squashed and merged into the 5.3 branch instead.Discussion----------[Translation] Introduce Translation ProvidersDocs forsymfony/symfony#38475,symfony/symfony#40926,symfony/symfony#40927, andsymfony/symfony#40947Ready for first review, but I'm not sure that I've written documentation in the right and all required places.ATM, Translation Providers Bridges packages doesn't exists, so Flex recipes are not created yet.Commits-------943a63f [Translation] Introduce Translation Providers
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@welcoMatticwelcoMatticwelcoMattic left review comments

@OskarStarkOskarStarkOskarStark left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

8 participants

@andrii-bodnar@carsonbot@fabpot@nicolas-grekas@stof@welcoMattic@OskarStark@Nyholm
[8]ページ先頭
©2009-2025 Movatter.jp