Skip to content

Support for PHP 5.3, SK bank code validation and throwing validation exceptions#1

Open
mbartok wants to merge 3 commits intobazo:masterfrom
mbartok:php53
Open

Support for PHP 5.3, SK bank code validation and throwing validation exceptions#1
mbartok wants to merge 3 commits intobazo:masterfrom
mbartok:php53

Conversation

@mbartok
Copy link

@mbartok mbartok commented Mar 6, 2015

Added validation of Slovak bank codes
Added support for php 5.3
Added dependency on php 5.3 to composer.json

@bazo
Copy link
Owner

bazo commented Mar 6, 2015

why would i want to support php 5.3? this version is dead

@mbartok mbartok changed the title Support for PHP 5.3 and Slovak bank code validation Support for PHP 5.3, SK bank code validation and throwing validation exceptions Mar 6, 2015
@mbartok
Copy link
Author

mbartok commented Mar 6, 2015

PHP 5.3 is still latest stable version on many obsolete hosting servers (unfortunately in my case) or is simply required due some backward compatibility in old applications. I don't thing that replacing array() to [] will somehow make code better or faster or more stable.

Also i've just commited major change to generate() method - throwing validation exception. It was too bad, that user couldn't be notified, why there was a problem by generating iban. Now the code is more reusable, you just have to add a try-catch block on generate() call.

@bazo
Copy link
Owner

bazo commented Mar 6, 2015

and why did you change the coding standard? now the pull request is a big mess with hard to track changes

@bazo
Copy link
Owner

bazo commented Mar 6, 2015

and php 5.3 needs to die. change your hosting. even websupport supports at least 5.5

@mbartok
Copy link
Author

mbartok commented Mar 6, 2015

I totally agree that php 5.3 needs to die but i have a important client and i simply cannot dictate these things. So what if your lib would be little bit more backward compatible? I didn't add any deprecated code.

Sorry for those indentations - i've used auto code formatting.

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