Skip to content

[Contracts] move Translatable to symfony/translation-contracts #38328

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

Closed

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Sep 28, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #38319
License MIT
Doc PR -

As discussed in the RFC:

  • Translatable is moved to symfony/contracts;
  • Translatable is not final anymore, it makes no sense for a value-object, especially in contracts;
  • the t() function is kept in the component - it doesn't fit in the contracts IMHO;
  • Translatable::trans() is not static anymore;
  • the domain is nullable instead of defaulting to "messages";
  • the t() function moved in the Symfony\Component\Translation namespace (for reference, the s() function from String is also namespaced.)

@nicolas-grekas
Copy link
Member Author

/cc @natewiebe13 FYI

@nicolas-grekas
Copy link
Member Author

FYI, I just moved the t() function in the Symfony\Component\Translation namespace (for reference, the s() function from String is also namespaced.)

fabpot added a commit that referenced this pull request Sep 28, 2020
This PR was merged into the 5.2-dev branch.

Discussion
----------

[lock] Fix non-blocking store fallback

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | /
| License       | MIT
| Doc PR        | /

Fix issue introduced in #38328

Tests added to avoid regressions.

Commits
-------

7a80e41 Fix non-blocking store fallback
@nicolas-grekas
Copy link
Member Author

See alternative proposal in #38330.
It's one or the other :)

@stof
Copy link
Member

stof commented Sep 28, 2020

@nicolas-grekas maybe a separate PR could be created (and merged) with all the common changes (nullable domain, namespaced t(), non static trans() method, etc...), leaving the 2 alternative PRs with only the changes related to their own approach (this depends how long the discussion about alternatives will take).

On a side note, merging any of these PRs will require updating the blog post as it changes the API.

@nicolas-grekas
Copy link
Member Author

Closing in favor of #38330 since that's where the consensus seems to go.

@nicolas-grekas nicolas-grekas deleted the contracts-translatable branch September 28, 2020 15:49
fabpot added a commit that referenced this pull request Sep 29, 2020
This PR was merged into the 5.2-dev branch.

Discussion
----------

[Contracts] add TranslatableInterface

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | #38319
| License       | MIT
| Doc PR        | -

An alternative to #38328:

- `TranslatableInterface` is added to symfony/contracts;
- `Translatable`, still in the component, implements it (and is not final as it makes no sense for a value-object)
- the `t()` function is kept in the component - it doesn't fit in the contracts IMHO;
- `Translatable::trans()` is not static anymore;
- the domain is nullable instead of defaulting to "messages";
- the `t()` function moved in the `Symfony\Component\Translation` namespace (for reference, the `s()` function from String is also namespaced.);
- the Twig extension is made strictly polymorphic: if you pass a Translatable, you cannot pass arguments/domain/count.

Commits
-------

9224f7a [Contracts] add TranslatableInterface
@nicolas-grekas nicolas-grekas removed this from the next milestone Oct 5, 2020
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.

[RFC] Move Translatable to contracts
4 participants