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

[SYMFONY] Add ability to configure slugify #104

Merged
merged 1 commit into from
Mar 30, 2016
Merged

[SYMFONY] Add ability to configure slugify #104

merged 1 commit into from
Mar 30, 2016

Conversation

estahn
Copy link
Contributor

@estahn estahn commented Mar 29, 2016

  • Moved the CocurSlugifyExtension into the DependencyInjection folder to comply with Symfony's bundle standard
  • Added configuration for the slugify service (e.g. enable lowercase)

@florianeckerstorfer
Copy link
Member

I don't think that is necessary. The Slugify bundle only contains three files, how I don't think we need to move them to separate namespaces.

@estahn
Copy link
Contributor Author

estahn commented Mar 29, 2016

@florianeckerstorfer Alright, i will move them all into one.

@estahn
Copy link
Contributor Author

estahn commented Mar 29, 2016

@florianeckerstorfer Actually it would be better to follow the symfony convention. The loader doesn't pick the config atm and the extension has been initialised in a wrong way.

I will adjust that now manually, but in future it would be better to stick to conventions.

@estahn
Copy link
Contributor Author

estahn commented Mar 29, 2016

@florianeckerstorfer Seems all good now. Do you need me to squash commits or is all good?

@florianeckerstorfer
Copy link
Member

Since these are a lot of commits, yes, please squash.

[SYMFONY] Add ability to configure slugify

[SYMFONY] Revert separation into DependencyInjection directory

[SYMFONY] Fix wrongly initialised extension

[SYMFONY] @Covers for CocurSlugifyExtensionTest::load

[SYMFONY] Adjust CocurSlugifyBundleTest
@estahn
Copy link
Contributor Author

estahn commented Mar 30, 2016

@florianeckerstorfer done.

@florianeckerstorfer florianeckerstorfer merged commit c2e35d3 into cocur:master Mar 30, 2016
@florianeckerstorfer
Copy link
Member

Thanks a lot.

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.

2 participants