Many slug fields in sluggable behavior

[dinoweb]

Hi, I needed a way to have more the one slug field in my entities or documents.
I've implemented a solution to this problem. Here is the code.
I've just added a property "Doctrine\Common\Annotations\Annotation\Sluggable::$slugField" set by default to "slug".
This tell to the SluggableListener which slug field to use.

I've created a new test for this: TranslatableManySlugTest

I tried not to break the compatibility with old notations, all the tests are running ok. May be there is a better way to achieve this goal.
Just have a look to my solution if you have time.

Thank you

[stof]

You should not add blank line here (and blank line should be really blank anyway whereas these one contain trailing whitespaces)

[stof]

You have the same sort of CS issues in many places.

[dinoweb]

Yes I know... I'll fix it as soon as i can. But what to do you think
about the implementation? The aidea?

Thanks
Lorenzo

On Wed, Jul 13, 2011 at 2:48 PM, stof
reply@reply.github.com
wrote:

You have the same sort of CS issues in many places.

Reply to this email directly or view it on GitHub:
https://github.com/l3pp4rd/DoctrineExtensions/pull/110#issuecomment-1562342

Lorenzo Caldara
Tel: +39 339 3577199
http://www.dinoweb.net
http://www.gemmyx.com/

[stof]

I don't know the Sluggable implementation well so I will let @l3pp4rd review this.

Btw, you should clean the PR: you have an old commit in the same branch that should not be part of it. Use a different branch for each pull request, creating them from the upstream branch to have a clean one.

[dinoweb]

Ok. I'm fixing all coding stiles problem. I'm new to git and githb so
thanks for your help.

Regards.

On Wed, Jul 13, 2011 at 2:58 PM, stof
reply@reply.github.com
wrote:

I don't know the Sluggable implementation well so I will let @l3pp4rd review this.

Btw, you should clean the PR: you have an old commit in the same branch that should not be part of it. Use a different branch for each pull request, creating them from the upstream branch to have a clean one.

Reply to this email directly or view it on GitHub:
https://github.com/l3pp4rd/DoctrineExtensions/pull/110#issuecomment-1562390

Lorenzo Caldara
Tel: +39 339 3577199
http://www.dinoweb.net
http://www.gemmyx.com/

[l3pp4rd]

Hi, I will review this when I can, thanks

[stof]

@dinoweb Here is the way to create a branch multiple_slugs with just the needed commit (assuming that the remote repo name is upstream for @l3pp4rd's one and origin for your fork):

git fetch upstream
git checkout -b multiple_slugs upstream/master
git cherry-pick 8a23176d
git push origin multiple_slugs

This will get your commit and apply it in a new multiple_slugs branch and then push this branch to github. You will then be able to open a new clean pull request from this branch (and fixing the CS issues in this branch).

The big drawback of using your master branch to send pull requests is that you create it on top of the previous one, which is an issue when the previous one was rejected (or when you need to create several independent pull requests). A pull request should always be created based on the current state of the code.

[dinoweb]

A quick question:

the code in "git cherry-pick" command is the last commit id in upstream/master?

Thanks
Lorenzo

On Wed, Jul 13, 2011 at 3:13 PM, stof
reply@reply.github.com
wrote:

@dinoweb Here is the way to create a branch multiple_slugs with just the needed commit (assuming that the remote repo name is upstream for @l3pp4rd's one and origin for your fork):

git fetch upstream
git checkout -b multiple_slugs upstream/master
git cherry-pick 8a23176d
git push origin multiple_slugs

This will get your commit and apply it in a new multiple_slugs branch and then push this branch to github. You will then be able to open a new clean pull request from this branch (and fixing the CS issues in this branch).

The big drawback of using your master branch to send pull requests is that you create it on top of the previous one, which is an issue when the previous one was rejected (or when you need to create several independent pull requests). A pull request should always be created based on the current state of the code.

Reply to this email directly or view it on GitHub:
https://github.com/l3pp4rd/DoctrineExtensions/pull/110#issuecomment-1562476

Lorenzo Caldara
Tel: +39 339 3577199
http://www.dinoweb.net
http://www.gemmyx.com/

[stof]

@dinoweb It is the sha1 of the commit you want to move to the new branch (I copied the one with the message Implemented a way to have many slug fields in Sluggable behavior). As you now have several commits you need to do this several times for the next ones.

[dinoweb]

I'm doing some disaster in my git repository. I'll send a new pull request as soon as I'll fix it.
Sorry for this.