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
Feature/1713 - Switch slugify to symfony string #1718
base: 4.x
Are you sure you want to change the base?
Feature/1713 - Switch slugify to symfony string #1718
Conversation
1325fce
to
7c87149
Compare
56dcb0a
to
dd239c4
Compare
- Refactored fixUrl method and created more unit tests. Issue sonata-project#1713
dd239c4
to
708060f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this last change we should be good. Thanks
@@ -35,7 +37,7 @@ final class PageManager extends BaseEntityManager implements PageManagerInterfac | |||
public function __construct( | |||
string $class, | |||
ManagerRegistry $registry, | |||
private SlugifyInterface $slugify, | |||
private SlugifyInterface|SluggerInterface $slugger, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add a deprecation if the slugger passed is not instance of SluggerInterface.
And a next major comment to reduce the param to sluggerInterface.
$slug = $page->getSlug(); | ||
|
||
// NEXT_MAJOR: Remove Slugify support | ||
if (null === $slug && $this->slugger instanceof Slugify) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (null === $slug && $this->slugger instanceof Slugify) { | |
if (null === $slug && $this->slugger instanceof SlugifyInterface) { |
no?
and might be better to write
if (null === $slug && $this->slugger instanceof Slugify) { | |
if (null === $slug && !$this->slugger instanceof SluggerInterface) { |
if (null === $slug && $this->slugger instanceof Slugify) { | ||
@trigger_error(sprintf( | ||
'%s is deprecated since version 4.8.0 and will be removed in 5.0, use %s instead of.', | ||
Slugify::class, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slugify::class, | |
SlugifyInterface::class, |
Replace slugify to symfony string
Closes #1713.
Changelog