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

Prevent eager loading hundreds of countries #14286

Merged
merged 1 commit into from Dec 12, 2023
Merged

Conversation

kayue
Copy link
Contributor

@kayue kayue commented Sep 7, 2022

Q A
Branch? 1.11 or 1.12
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Related tickets #8632
License MIT

Eager loading all countries would add extra 8ms - 10ms overhead to Sylius' boot up time. Creating a draft PR to see what test case would it break first.

@kayue kayue marked this pull request as ready for review September 8, 2022 01:38
@kayue kayue requested a review from a team as a code owner September 8, 2022 01:38
@kayue
Copy link
Contributor Author

kayue commented Sep 8, 2022

The broken tests seems not related to the changes, not sure what are they about.

@vvasiloi
Copy link
Contributor

vvasiloi commented Sep 8, 2022

@kayue those are related to the recent deprecations in Symfony Mailer. A new version was tagged less than an hour before your PR: https://github.com/Sylius/SyliusMailerBundle/releases/tag/v1.8.0

@kayue
Copy link
Contributor Author

kayue commented Sep 10, 2022

Rebasing this branch and hopefully tests will pass this time.

This change is working in my local project so I am confident that it will pass the tests.

@kayue
Copy link
Contributor Author

kayue commented Sep 10, 2022

All tests are passing now, thanks.

@coldic3 coldic3 added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Mar 9, 2023
@probot-autolabeler probot-autolabeler bot added Docker Docker-related issues and PRs. Documentation Documentation related issues and PRs - requests, fixes, proposals. Maintenance CI configurations, READMEs, releases, etc. labels May 20, 2023
@jakubtobiasz jakubtobiasz changed the base branch from 1.12 to 1.13 May 20, 2023 15:20
@NoResponseMate NoResponseMate removed Documentation Documentation related issues and PRs - requests, fixes, proposals. Docker Docker-related issues and PRs. labels May 25, 2023
Copy link

github-actions bot commented Dec 7, 2023

Bunnyshell Preview Environment deleted

Available commands:

  • /bns:deploy to redeploy the environment

@jakubtobiasz jakubtobiasz merged commit 05d55a6 into Sylius:1.13 Dec 12, 2023
25 checks passed
@jakubtobiasz
Copy link
Member

Thank you, @kayue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants