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

Optimize "see more" action on navigation filter #3201

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

thomas-kl1
Copy link
Contributor

No description provided.

@romainruaud
Copy link
Collaborator

How could the call to $this->categoryRepository->create() work if this is not a factory anymore but the repository directly ?

@thomas-kl1
Copy link
Contributor Author

Whoops missed it in my copy paste!

@romainruaud
Copy link
Collaborator

Could you please squash your commits and reduce their content to the modification related to the CategoryRepository ? You modified the cosmetic of the whole file but it's not necessary and merging it as is would flag too much lines as being concerned by the CategoryRepository refactoring.

Regards

@thomas-kl1 thomas-kl1 changed the title Use shared instance of Category repository Optimize "see more" action on navigation filter Mar 1, 2024
@thomas-kl1 thomas-kl1 force-pushed the patch-1 branch 2 times, most recently from daf1dd6 to ad107ca Compare March 1, 2024 15:35
@thomas-kl1
Copy link
Contributor Author

@romainruaud I just rephrased the PR and squashed the commit. I've added comments regarding the changes.

@romainruaud
Copy link
Collaborator

Thanks, can you fix the PHPCs violations so that we can start reviewing and testing this ?

Best regards

@rbayet rbayet requested a review from romainruaud March 14, 2024 08:13
@romainruaud romainruaud requested review from rbayet and removed request for romainruaud March 14, 2024 08:58
@romainruaud romainruaud assigned rbayet and unassigned romainruaud Mar 14, 2024
@rbayet rbayet merged commit 36a53ed into Smile-SA:2.11.x Mar 14, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants