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.

Sign up for GitHub

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

Jump to bottom

[Channel][Currency][Locale][Money] Clean up components and bundles #6383

Merged
merged 8 commits into from
Oct 12, 2016
Merged

[Channel][Currency][Locale][Money] Clean up components and bundles #6383

merged 8 commits into from
Oct 12, 2016

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Oct 11, 2016 •
edited
Loading

Q A
Bug fix? yes
New feature? no
BC breaks? yes
Related tickets
License MIT
  • Channel
  • ChannelBundle
  • Currency
  • CurrencyBundle
  • Locale
  • LocaleBundle
  • MoneyBundle

@pjedrzejewski pjedrzejewski added the Maintenance CI configurations, READMEs, releases, etc. label Oct 11, 2016
@pjedrzejewski
Copy link
Member

Why renaming sylius.context.currency with sylius.currency_context? I think we can identify a common group of "context" services in Sylius. I think it was good DX to know that you can always look for sylius.context.X service to get something. I understand it is a bit against my arguments for grouping services (you can't really swap contexts, they do not share an interface), but I have a feeling that was a good naming convention in terms of DX. Any thoughts?

@GSadee
Copy link
Member Author

GSadee commented Oct 12, 2016

I've been suggested by what we were talking about earlier, but of course I understand your arguments and what is more, I agree with them 😃 So I'll revert these changes.

@GSadee GSadee changed the title [WIP][Channel][Currency][Locale][Money] Clean up [Channel][Currency][Locale][Money] Clean up Oct 12, 2016
@GSadee GSadee changed the title [Channel][Currency][Locale][Money] Clean up [Channel][Currency][Locale][Money] Clean up components and bundles Oct 12, 2016
@pjedrzejewski pjedrzejewski merged commit 12973c3 into Sylius:master Oct 12, 2016
@pjedrzejewski
Copy link
Member

Thank you Grzesiu!

@GSadee GSadee deleted the channel-related-clean-up branch September 22, 2017 08:25
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
@pjedrzejewski
Merge pull request Sylius#6383 from GSadee/channel-related-clean-up
fc0ca2e 
[Channel][Currency][Locale][Money] Clean up components and bundles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GSadee @pjedrzejewski