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

[FrameworkBundle] [Command] TranslationUpdate change default output to xlf #27935

Conversation

R2c
Copy link

@R2c R2c commented Jul 12, 2018

Q A
Branch? master
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets Not applicable
License MIT
Doc PR Not applicable

Simple modification on the default output for the translation: update command to XLIFF (xlf)
It's to be in agreement with the documentation

image

Link to the documentation :
https://symfony.com/doc/master/translation.html#translation-resource-file-names-and-locations

@R2c R2c changed the title [FrameworkBundle] Cmd TranslationUpdate change default output to xlf [FrameworkBundle] [Command] TranslationUpdate change default output to xlf Jul 12, 2018
@javiereguiluz
Copy link
Member

@R2c thanks for sending your first contribution to Symfony!

I like this change a lot ... but sadly it's a backward compatibility break. Any existing command that doesn't specify the format gets the output in YML. If we make this change, the output will be XLIFF and things could break.

If we want to make this change, we need to find a way of doing it without breaking existing commands. Thanks!

@R2c
Copy link
Author

R2c commented Jul 13, 2018

I searched in Symfony, i don't find another command with output format in YML.
Maybe i'm wrong, but i think this command is clearly manual (dev).
I don't think people use this command in production, with crontab, etc.
That's why I put it on master.

I'm opened to make more changes, any idea about the compatibility ?

  • Adding message on the changelog ?
  • Adding message on the command... Starting at 3.4 - Output format will change in 4.2 to xliff

@ro0NL
Copy link
Contributor

ro0NL commented Jul 13, 2018

given ppl expect yaml format today, and with this change, should use format=yml in the future... we could deprecate not passing a format, so we can change the default in 5.0

not sure if worth it :)

@fabpot
Copy link
Member

fabpot commented Jul 18, 2018

As this command is indeed something people run manually, I don't think changing the default is worth it.

@R2c
Copy link
Author

R2c commented Jul 19, 2018

My constat was :

Starting to use SF4 from the skeleton to look / try every changes. So I try a lot of stuff, when i try to install/use the translation part, i was suprise to see a .yml file? it's not .yaml in SF4 🤔

I use SF since a lot of years, but i'm imagining someone who start with it, i'm sure he will not understand why and it's look like a bug.

If we don't change now, it'll never change.

@nicolas-grekas nicolas-grekas added this to the next milestone Jul 23, 2018
@nicolas-grekas
Copy link
Member

I don't see any simple way to change the default value without any BC break either. Which means that yes, it should stay as is forever.
We could still allow passing "yaml" as output-format?

@fabpot
Copy link
Member

fabpot commented Aug 28, 2018

What do we do here? I don't think we have a non-BC break policy on commands (I'm pretty sure we broke some in the past). Documenting the change in the CHANGELOG could be enough?

@fabpot
Copy link
Member

fabpot commented Sep 4, 2018

Thank you @R2c.

@fabpot fabpot merged commit 137593e into symfony:master Sep 4, 2018
fabpot added a commit that referenced this pull request Sep 4, 2018
…efault output to xlf (Alexis BOYER)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[FrameworkBundle] [Command] TranslationUpdate change default output to xlf

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | Not applicable
| License | MIT
| Doc PR | Not applicable

Simple modification on the default output for the translation: update command to XLIFF (xlf)
It's to be in agreement with the documentation

![image](https://user-images.githubusercontent.com/2004449/42637935-a91dcabc-85ec-11e8-86db-9c8bea5e710b.png)

Link to the documentation :
https://symfony.com/doc/master/translation.html#translation-resource-file-names-and-locations

Commits
-------

137593e [FrameworkBundle] Cmd TranslationUpdate change default output to xlf
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
This was referenced Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants