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

Fix service definition at custom-payment-gateway #14554

Merged
merged 1 commit into from
Nov 29, 2022
Merged

Fix service definition at custom-payment-gateway #14554

merged 1 commit into from
Nov 29, 2022

Conversation

diimpp
Copy link
Member

@diimpp diimpp commented Nov 17, 2022

Q A
Branch? 1.11
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets fixes #13915
License MIT

@diimpp diimpp requested a review from a team as a code owner November 17, 2022 19:46
@probot-autolabeler probot-autolabeler bot added the Documentation Documentation related issues and PRs - requests, fixes, proposals. label Nov 17, 2022
@jakubtobiasz
Copy link
Contributor

Hi @diimpp!
Why do you think this service should be marked as public?

@diimpp
Copy link
Member Author

diimpp commented Nov 18, 2022

Hello @jakubtobiasz,

I cannot tell you exact reason why custom payum actions should be registered as public, but that's the way with SyliusPayumBundle custom actions

Sylius/src/Sylius/Bundle/PayumBundle/Resources/config/services/action.xml

Line 8 in 23dc8d6

<defaults public="true" />

as well in my personal experience I was forced to do the same for my custom actions.

There are two service definitions in this doc, xml and yaml. Xml contains correct public definition, while yaml misses it. (See related ticket)

@diimpp
Copy link
Member Author

diimpp commented Nov 28, 2022

@jakubtobiasz hi, what can be done to merge it early? Do you need second review for core team?

@jakubtobiasz
Copy link
Contributor

@diimpp yep, more votes needed, I pinged the team.

Zales0123
Zales0123 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Member

@Zales0123 Zales0123 left a comment

Choose a reason for hiding this comment

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

🖖

@Zales0123 Zales0123 merged commit b2361c1 into Sylius:1.11 Nov 29, 2022
@Zales0123
Copy link
Member

Thank you, Dmitri! 🎉

@diimpp diimpp deleted the patch-18 branch November 29, 2022 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zales0123 Zales0123 Zales0123 approved these changes

@jakubtobiasz jakubtobiasz jakubtobiasz approved these changes

Assignees
No one assigned
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@diimpp @jakubtobiasz @Zales0123