8000 [Translation] Add `--as-tree` option to `translation:pull` command by syffer · Pull Request #51153 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Translation] Add --as-tree option to translation:pull command #51153

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.

Sign up for GitHub

By clicking “Sign up f 8000 or 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

Jump to bottom
Merged
merged 1 commit into from
Sep 20, 2023
Merged

[Translation] Add --as-tree option to translation:pull command #51153

merged 1 commit into from
Sep 20, 2023

Conversation

syffer
Copy link
Contributor
@syffer syffer commented Jul 28, 2023
Q A
Branch? 6.4
Bug fix? yes/no
New feature? yes
Deprecations? no
Tickets n/a
License MIT
Doc PR todo

This PR adds an option --as-tree to the command translation:pull, like the PR #38393 did it for the command translation:extract.

This option would ease the use of the command translation:pull when the yaml translations are stored as a tree. Without it, the yaml translations are inlined after a translation:pull, regardless of whether it was stored as inline or as a tree. To bypass this, you would either have to

  • re-indent the yaml file yourself after a translation:pull
  • replace one service (e.g. translation.dumper.yaml) to forcefully add the option as_tree for the yaml file dumper

@carsonbot carsonbot added this to the 6.4 milestone Jul 28, 2023
@carsonbot
Copy link

Hey!

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

Symfony has a contribution guide which I sugge 8000 st you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.4 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

Nyholm
Nyholm reviewed Jul 29, 2023
View reviewed changes
Copy link
Member
@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Lovely. Well done.

I like that you added tests for this feature.
I had a few comments that I would like your help addressing.

@OskarStark OskarStark changed the title [Translation] Add option --as-tree to translation:pull command [Translation] Add --as-tree option to translation:pull command Aug 2, 2023
welcoMattic
welcoMattic approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member
@welcoMattic welcoMattic left a comment

Choose a reason for hiding this comment

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

Little comment on the changelog, otherwise it's ok for me 👍. Thanks for your contribution

@nicolas-grekas
Copy link
Member

Thank you @syffer.

@nicolas-grekas nicolas-grekas merged commit f9327be into symfony:6.4 Sep 20, 2023
@nicolas-grekas nicolas-grekas mentioned this pull request Sep 20, 2023
Closed
This was referenced Oct 21, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot left review comments

@Nyholm Nyholm Nyholm left review comments

@welcoMattic welcoMattic welcoMattic approved these changes

Assignees
No one assigned
Labels
Bug Feature Status: Needs Review Translation
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
6 participants
@syffer @carsonbot @nicolas-grekas @fabpot @welcoMattic @Nyholm
0