Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Validator] Improved ISBN validator#10542
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
must be removed
fabpot commentedMar 27, 2014
This changes can only be done in the master branch. |
fabpot commentedMar 27, 2014
Deprecations can happen in any versions (here it would be 2.5). |
sprain commentedMar 27, 2014
Ok, I expected this. I will close this and create a new one on master branch. |
sprain commentedMar 27, 2014
Deprecations: How will be taken care of removing the deprecated code? |
This PR was merged into the 2.5-dev branch.Discussion----------[Validator] Improved ISBN validator| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | yes| Fixed tickets |#10386,#10387,#10542| License | MIT| Doc PR |symfony/symfony-docs/pull/3724This is a new PR of#10542, now on master branch.Todos:- [x] Discuss and determine deprecation versions- [x] Update docsAfter following some discussion in the tickets mentioned above I have improved the ISBN validator, which has had some inconsistencies:* Use a `type` which can be set to `isbn10` or `isbn13` or `null` (checks if value matches any type) instead of the current boolean `isbn10` and `isbn13` options which cause confusion (e.q. if both are true, does this mean the value must match any or both? You could think it's the latter, but that's actually impossible.).* In the IBAN validator we recently agreed to be strict about upper- and lowercase handling (#10489). Therefore this should be also the case with the ISBN validator. Some ISBN10 numbers may end with an uppercase `X` (representing the check-digit 10), while a lowercase `x` is considered wrong (see [here](http://www.goodreads.com/topic/show/1253500-a-question-about-isbn-10-s---ending-in-x) and [here](http://en.wikipedia.org/wiki/Category:Pages_with_ISBN_errors)). I did not have access to the actual specifications as I have only found documentation which costs about $100 (e.q. [here](http://www.iso.org/iso/catalogue_detail?csnumber=36563)).To avoid bc breaks I suggest to introduce deprecations for current constraint options. [In the documentation](http://symfony.com/doc/current/contributing/code/conventions.html#deprecations) I haven't found any information about which versions may introduce deprecations, so you might have to help me out here with hints on how to handle it correctly. I'll be happy to provide the code with the deprecated parts removed after that.Commits-------ec42844 Improved ISBN validator
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.
should be a strict comparison
Todos:
After following some discussion in the tickets mentioned above I have improved the ISBN validator, which has had some inconsistencies:
typewhich can be set toisbn10orisbn13ornull(checks if value matches any type) instead of the current booleanisbn10andisbn13options which cause confusion (e.q. if both are true, does this mean the value must match any or both? You could think it's the latter, but that's actually impossible.).X(representing the check-digit 10), while a lowercasexis considered wrong (seehere andhere). I did not have access to the actual specifications as I have only found documentation which costs about $100 (e.q.here).To avoid bc breaks I suggest to introduce deprecations for current constraint options.In the documentation I haven't found any information about which versions may introduce deprecations, so you might have to help me out here with hints on how to handle it correctly. I'll be happy to provide the code with the deprecated parts removed after that.