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

[Theme] Context interface cleanup #4156

Merged
merged 2 commits into from
Feb 16, 2016

Conversation

pamil
Copy link
Contributor

@pamil pamil commented Feb 15, 2016

Q A
Bug fix? no
New feature? no
BC breaks? yes (within master)
Deprecations? no
Fixed tickets -
License MIT
Doc PR -
  • Removed ThemeContextInterface::getThemeHierarchy() proxy/helper method
  • Removed ThemeContextInterface::setTheme(ThemeInterface $theme) method as it does not apply to all theme contexts out there

ThemeContextInterface has only a getTheme() method now, which will be a lot easier to implement and maintain.

@pjedrzejewski
Copy link
Member

All contexts ultimately should have only get* method. :) 👍

@pjedrzejewski pjedrzejewski added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). DX Issues and PRs aimed at improving Developer eXperience. labels Feb 15, 2016
@pamil pamil force-pushed the theme-bundle-context-cleanup branch from 31f48d0 to 8ff53d1 Compare February 16, 2016 12:40
pjedrzejewski pushed a commit that referenced this pull request Feb 16, 2016
@pjedrzejewski pjedrzejewski merged commit 55daec8 into Sylius:master Feb 16, 2016
@pjedrzejewski
Copy link
Member

Thanks Kamil!

@pamil pamil deleted the theme-bundle-context-cleanup branch February 16, 2016 14:17
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Issues and PRs aimed at improving Developer eXperience. Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants