Skip to content

Removing unused service argument #7169

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

Merged
merged 3 commits into from
Oct 16, 2023

Conversation

mamazu
Copy link
Contributor

@mamazu mamazu commented Oct 2, 2023

Q A
Bug fix? no
New feature? no
BC breaks? yes
License MIT

What's in this PR?

Why?

The property is not used anywhere and instanciating the service is quite expensive so this should also improve the container speed a little.

To Do

  • Add breaking changes to UPGRADE.md

@mamazu mamazu force-pushed the unused_service_argument branch 4 times, most recently from 7d522c3 to a42d8e5 Compare October 2, 2023 18:25
@mamazu mamazu force-pushed the unused_service_argument branch from a42d8e5 to 0c36bc4 Compare October 2, 2023 18:30
@alexander-schranz alexander-schranz added the hacktoberfest-accepted Accepted for hacktoberfest but will be merged later label Oct 16, 2023
@alexander-schranz alexander-schranz added the DX Affecting the end developer label Oct 16, 2023
@mamazu
Copy link
Contributor Author

mamazu commented Oct 16, 2023

Should we also make a fix for the lower versions and mark the content manager as lazy so that it get only instantiated when it's needed?

@alexander-schranz
Copy link
Member

@mamazu think marking the sulu.content.mapper service as lazy should not hurt 🤔

@alexander-schranz alexander-schranz merged commit 6621a42 into sulu:3.0 Oct 16, 2023
@mamazu mamazu deleted the unused_service_argument branch October 17, 2023 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Affecting the end developer hacktoberfest-accepted Accepted for hacktoberfest but will be merged later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants