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

[AdminBundle,GeneratorBundle,NodeBundle,UtilitiesBundle] Change slugifier into service #404

Merged
merged 1 commit into from May 26, 2015

Conversation

Projects
None yet
4 participants
@woutervandamme
Copy link
Contributor

woutervandamme commented May 12, 2015

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets #84

@woutervandamme woutervandamme force-pushed the woutervandamme:feature/slugifier branch from e264923 to c7a24d5 May 12, 2015

@krispypen

This comment has been minimized.

Copy link
Member

krispypen commented May 12, 2015

@woutervandamme check the travis build failure
also create a SlugifierInterface and add information then how to create your own slugifier (by extending the default one or implementing the interface)

@@ -296,7 +296,7 @@ public function createNodeTranslationFor(HasNodeInterface $hasNode, $lang, Node
->setNode($node)
->setLang($lang)
->setTitle($hasNode->getTitle())
->setSlug(Slugifier::slugify($hasNode->getTitle(), ''))
->setSlug($hasNode->getTitle(), '')

This comment has been minimized.

@jockri

jockri May 12, 2015

Member

This is not correct

@woutervandamme woutervandamme force-pushed the woutervandamme:feature/slugifier branch 4 times, most recently from 7ae7e88 to a83d070 May 13, 2015

@@ -8,6 +8,18 @@
class UtilitiesTwigExtension extends Twig_Extension
{
/**
* @var Slugifier

This comment has been minimized.

@krispypen

krispypen May 13, 2015

Member

Use the interface here

@@ -9,20 +9,32 @@
*/
class SlugifierTest extends \PHPUnit_Framework_TestCase
{
/**
* @var Slugifier

This comment has been minimized.

@krispypen

krispypen May 13, 2015

Member

Use the interface here

@@ -26,14 +27,20 @@ class NodeTranslationListener
private $nodeTranslations;
/**
* @var Slugifier

This comment has been minimized.

@krispypen

krispypen May 13, 2015

Member

Use the interface here

@woutervandamme woutervandamme force-pushed the woutervandamme:feature/slugifier branch 2 times, most recently from 5151a46 to 81e0ed2 May 13, 2015

@roderik roderik added this to the 3.1.2 milestone May 26, 2015

@roderik

This comment has been minimized.

Copy link
Contributor

roderik commented May 26, 2015

@woutervandamme please rebase on top of master, thanks!

woutervandamme
changed slugifier into service
added slugifier service to fixtures

add slugifier service to NodeBundle

updated slugifier tests

inject slugifier into utilitiesTwigExtension

twigExtension now works with the slugifier service

removed @Covers in slugifierTest

implemented slugifierInterface

@woutervandamme woutervandamme force-pushed the woutervandamme:feature/slugifier branch from 81e0ed2 to 1541fce May 26, 2015

@krispypen

This comment has been minimized.

Copy link
Member

krispypen commented May 26, 2015

code review: ok

roderik pushed a commit that referenced this pull request May 26, 2015

Roderik van der Veer
Merge pull request #404 from woutervandamme/feature/slugifier
[AdminBundle,GeneratorBundle,NodeBundle,UtilitiesBundle] Change slugifier into service

@roderik roderik merged commit 92ebd88 into Kunstmaan:master May 26, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment