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

[Locale] Removed enable/disable functionality #7346

Merged

Conversation

NoResponseMate
Copy link
Contributor

@NoResponseMate NoResponseMate commented Jan 25, 2017

Q A
Bug fix? more of a precaution
New feature? no
BC breaks? yes
Related tickets explained in, closes #7309
License MIT

@pjedrzejewski pjedrzejewski added the BC Break PRs introducing BC breaks (do not even try to merge). label Jan 25, 2017
locale_de:
code: de
enabled: false
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to have de locale defined separately anymore.

@@ -59,23 +52,13 @@ public function buildForm(FormBuilderInterface $builder, array $options)

$locale = $event->getData();
if ($locale instanceof LocaleInterface && null !== $locale->getCode()) {
$options['disabled'] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it shouldn't be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went a bit overboard on this one. 👌

@michalmarcinkowski michalmarcinkowski merged commit b703c99 into Sylius:master Jan 31, 2017
@michalmarcinkowski
Copy link
Contributor

Thanks Janek! 👍

@NoResponseMate NoResponseMate deleted the locale/remove-toggleability branch January 31, 2017 16:19
igormukhingmailcom added a commit to igormukhingmailcom-forks/Sylius that referenced this pull request Apr 14, 2019
@igormukhingmailcom igormukhingmailcom mentioned this pull request Apr 14, 2019
1 task
GSadee pushed a commit to igormukhingmailcom-forks/Sylius that referenced this pull request Apr 19, 2019
lchrusciel added a commit that referenced this pull request Apr 19, 2019
This PR was merged into the 1.3 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | 1.3
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | not sure (it backward compatible on default fixtures configuration)
| Deprecations?   | not sure (deprecation messages was added, but not appear on default fixtures configuration)
| Related tickets | fixes #10307, partially #7346
| License         | MIT

So, in BC in mind, with this PR it now will work like this:

- **If you have default (empty) configuration, as was before**:

    ```yaml
    sylius_fixtures:
        # ...
        fixtures:
            locale: ~
    ```
  Then, base locale **will be added** to configuration.

- *But if you provided a list of locales*:

    ```yaml
    sylius_fixtures:
        suites:
            my_custom_suite_to_add_to_existing_database:
                listeners:
                    orm_purger: false
                fixtures:
                    locale:
                        options:
                            locales:
                                - "de_DE"
    ```
   Then, base locale **will NOT be added** to configuration.

Deprecation message was added to avoid usage base locale at all in future. To suppress that message on default configuration, `"%locale%"` was passed directly to the `locales` option at `src/Sylius/Bundle/CoreBundle/Resources/config/app/fixtures.yml`.

# TODO

- [ ] Define Sylius version where base locale will be removed OR remove deprecation message if base locale should be kept by default (when fixture defined like `locale: ~`) - I'm not 100% sure about this so want to discuss this first.


Commits
-------

93f098e Fixed: LocaleFixtureTest as it not valid since #7346
15c0cb7 Avoided: Adding base locale usage to list of fixtures (fixes #10307)
lchrusciel added a commit that referenced this pull request Apr 19, 2019
* 1.3:
  Avoided: Adding base locale usage to list of fixtures (fixes #10307)
  Fixed: LocaleFixtureTest as it not valid since #7346
lchrusciel added a commit that referenced this pull request Apr 19, 2019
* 1.4:
  Avoided: Adding base locale usage to list of fixtures (fixes #10307)
  Fixed: LocaleFixtureTest as it not valid since #7346
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
…eability

[Locale] Removed enable/disable functionality
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break PRs introducing BC breaks (do not even try to merge).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Non Toggleable Locales
7 participants