cakephp / localized

I18n and L10n related CakePHP code
Other
213 stars 177 forks source link

Update FrValidation.php #87

Closed Back-Sephiroth closed 9 years ago

Back-Sephiroth commented 9 years ago

add phone numbers like : 07xxxxxxxx and 08xxxxxxxx and 09xxxxxxxx

antograssiot commented 9 years ago

looks good and necessary

dereuromark commented 9 years ago

Maybe you could also fix the cs errors pretty quick: https://travis-ci.org/cakephp/localized/jobs/40557670

Back-Sephiroth commented 9 years ago

Hello,

Sorry, but i don't understand what is the cs errors. I don't know how travis run,

yours sincerely

2014-11-10 18:51 GMT+01:00 Mark S. notifications@github.com:

Maybe you could also fix the cs errors pretty quick: https://travis-ci.org/cakephp/localized/jobs/40557670

— Reply to this email directly or view it on GitHub https://github.com/cakephp/localized/pull/87#issuecomment-62424206.

Robin FRERE

P Pensez à l'environnement avant d'imprimer ce message .

dereuromark commented 9 years ago

Coding Standards. Just two missing spaces. But I can do that after the merge, as well.

cake17 commented 9 years ago

@dereuromark if you want I can fix the spaces after your merge, or would you prefer I send a new PR?

Back-Sephiroth commented 9 years ago

Yes you can do it, because i don't really understand what happen wrong.

yours sincerely

2014-11-19 22:29 GMT+01:00 cake17 notifications@github.com:

@dereuromark https://github.com/dereuromark if you want I can fix the spaces after your merge, or would you prefer I send a new PR?

— Reply to this email directly or view it on GitHub https://github.com/cakephp/localized/pull/87#issuecomment-63717117.

Robin FRERE

P Pensez à l'environnement avant d'imprimer ce message .

ADmad commented 9 years ago

@dereuromark The CS error reported by travis is not introduced by this PR, so I think it's a bit over reaching to expect @Back-Sephiroth to fix it. The problem reported is in RuValidation.php while this PR only changes FrValidation.php.

cake17 commented 9 years ago

In fact, the problem is not in the french file

Can you edit the file in /localized/Validation/RuValidation.php 89 | ERROR | Expected 1 space after closure's function keyword; 0 found 104 | ERROR | Expected 1 space after closure's function keyword; 0 found

If you can't change, tell me again, I'll do it

ADmad commented 9 years ago

@Back-Sephiroth Don't worry about the coding standard errors, they are not caused by your changes :) Though it would be nice if you could add tests cases for your changes.

Back-Sephiroth commented 9 years ago

I have just do the modification,

I'll try to see if the travis test will pass

2014-11-19 22:42 GMT+01:00 ADmad notifications@github.com:

@Back-Sephiroth https://github.com/Back-Sephiroth Don't worry about the coding standard errors, they are not caused by your changes :) Though it would be nice if you could add tests cases for your changes.

— Reply to this email directly or view it on GitHub https://github.com/cakephp/localized/pull/87#issuecomment-63719222.

Robin FRERE

P Pensez à l'environnement avant d'imprimer ce message .

cake17 commented 9 years ago

Thanks ! :+1: Can you make the commit on your branch patch1 so the PR will be updated automatically ? I promess it will be my last demand :-)

Back-Sephiroth commented 9 years ago

Ok, i commit on patch1,

At first it was on master branch, i don't know how, but i change this one.

2014-11-19 22:57 GMT+01:00 cake17 notifications@github.com:

Thanks ! [image: :+1:] Can you make the commit on your branch patch1 so the PR will be updated automatically ?

— Reply to this email directly or view it on GitHub https://github.com/cakephp/localized/pull/87#issuecomment-63721572.

Robin FRERE

P Pensez à l'environnement avant d'imprimer ce message .

dereuromark commented 9 years ago

@Back-Sephiroth Nice work. Even though it wasn't a requirement (as you already knew anyway and as @ADmad said as well) it is nice that you got travis up to speed along with it.

But are you able to provide a test case for your modification which backs up the necessity to change that phone number rule?

cake17 commented 9 years ago

@Back-Sephiroth merci beaucoup !!

Back-Sephiroth commented 9 years ago

Cela m'a bien fait plaisir de vous aider. Et je continuerais à contribuer si je remarque des problèmes à ma portée.

Je découvre et je commence à bien maîtriser cake, merci pour ce magnifique boulot, j'adore.

Bonne journée

2014-11-20 9:26 GMT+01:00 cake17 notifications@github.com:

@Back-Sephiroth https://github.com/Back-Sephiroth merci beaucoup !!

— Reply to this email directly or view it on GitHub https://github.com/cakephp/localized/pull/87#issuecomment-63775095.

Robin FRERE

P Pensez à l'environnement avant d'imprimer ce message .