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

Organization of ConsoleCommands/Messages #15730

Conversation

Wojdylak
Copy link
Member

@Wojdylak Wojdylak commented Jan 15, 2024

Q A
Branch? 1.13
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Related tickets N/A
License MIT

All CLI commands → Console/Command
All Message → Command

List of not released clases:

  • \Sylius\Bundle\AdminBundle\Command\Factory\QuestionFactory
  • \Sylius\Bundle\AdminBundle\Command\Factory\QuestionFactoryInterface
  • \Sylius\Bundle\AdminBundle\Command\ChangeAdminUserPasswordCommand
  • \Sylius\Bundle\AdminBundle\Command\CreateAdminUserCommand

Due to validation issues (e.g., in src/Sylius/Bundle/ApiBundle/Resources/config/validation/AdminResetPassword.xml) that do not recognize aliases created on the class, I am unable to move classes from the Message, MessageDispatcher, and MessageHandler folders. I have only added information in the upgrade file that this change will be made in version 2.0.

@Wojdylak Wojdylak requested a review from a team as a code owner January 15, 2024 12:26
@probot-autolabeler probot-autolabeler bot added Admin AdminBundle related issues and PRs. Maintenance CI configurations, READMEs, releases, etc. labels Jan 15, 2024
Copy link

github-actions bot commented Jan 15, 2024

Bunnyshell Preview Environment deleted

Available commands:

  • /bns:deploy to redeploy the environment

@Wojdylak Wojdylak force-pushed the SYL-3157-organization-of-api-commands-messages branch 3 times, most recently from 8a3b47d to ba4f18e Compare January 16, 2024 19:17
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Jan 16, 2024
@Wojdylak Wojdylak force-pushed the SYL-3157-organization-of-api-commands-messages branch 3 times, most recently from 3421380 to 36c999e Compare January 17, 2024 15:01
@Wojdylak Wojdylak changed the title 🚧 Organization of ConsoleCommands/Messages Organization of ConsoleCommands/Messages Jan 17, 2024
jakubtobiasz
jakubtobiasz previously approved these changes Jan 18, 2024
@jakubtobiasz jakubtobiasz dismissed stale reviews from NoResponseMate and themself via 19f97c6 January 18, 2024 05:55
@jakubtobiasz jakubtobiasz force-pushed the SYL-3157-organization-of-api-commands-messages branch from 36c999e to 19f97c6 Compare January 18, 2024 05:55
@jakubtobiasz
Copy link
Member

I resolved the conflicts 🖖🏻.

@Wojdylak Wojdylak force-pushed the SYL-3157-organization-of-api-commands-messages branch from 19f97c6 to 7bd9755 Compare January 18, 2024 07:23
<tag name="console.command" />
</service>

<service id="Sylius\Bundle\UserBundle\Command\PromoteUserCommand">
<service id="Sylius\Bundle\UserBundle\Command\DemoteUserCommand" alias="Sylius\Bundle\UserBundle\Console\Command\DemoteUserCommand" />
Copy link
Member

Choose a reason for hiding this comment

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

What do you think adding deprecations to the aliases as well?

https://symfony.com/doc/current/service_container/alias_private.html#deprecating-service-aliases

Copy link
Member Author

Choose a reason for hiding this comment

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

The files contain information about deprecation, so I think it is enough.

@NoResponseMate NoResponseMate merged commit 55ed627 into Sylius:1.13 Jan 19, 2024
25 checks passed
@NoResponseMate
Copy link
Contributor

Thank you, @Wojdylak!

@Wojdylak Wojdylak deleted the SYL-3157-organization-of-api-commands-messages branch February 21, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin AdminBundle related issues and PRs. API APIs related issues and PRs. Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants