skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
119 stars 31 forks source link

Clearly announce that te libraries does not recognize RU as valid #118

Closed tdltdl closed 1 year ago

tdltdl commented 1 year ago

Describe the bug Commit 2d0da7a makes every Russian bank account invalid (I do not challenge your decision). This has been released as part of 5.7.2 (minor release), but does not appear on the release note and will potentially cause production problem for people updating without looking at every commit.

To Reproduce Read the release note of 5.7.2 and look at commit 2d0a7a

Expected behavior A visible warning in every release affected to inform this backward incompatible change : Russian accounts will be considered as INVALID as consequence of the Ukraine's war.

IbanNet library/version

.NET runtime NA

Operating system NA

Additional context The problem is that people might upgrade and face huge problem in production by not knowing that this ban has been put in place. As the author you are free to change the code as you want, but this might have serious consequences for people trusting your work.

skwasjer commented 1 year ago

\o

IbanNet never supported RU in earlier versions, so there is no regression caused by any (newer) versions to any older version with regards to this change and you can safely upgrade. This is why I did not deem it necessary to make a mention of it in release notes.

The country was added both to SWIFT and Wikipedia last year. As such, indeed, regenerating both providers would have resulted in inclusion of RU to a released version of IbanNet. But since I excluded it immediately, no version was ever released where RU was part of IbanNet, just look at the diffs/history of the providers themselves (not the .tt files) or the diffs/history of the supported countries file.

(SwiftRegistryProvider received the same treatment in https://github.com/skwasjer/IbanNet/commit/87e2abb125de66b1eb4af349f7a460a98992ea04)

That said, I don't mind putting in a notice on the supported countries page.

tdltdl commented 1 year ago

Hi Thanks for your fast answer. Didn't noticed that it was only recently added - however I think it is fair, if people know that your data source is both Wikipedia and Swift, to mention it explicitly. I spotted this just by reviewing the recent commits of the library, and was surprised it was not mentioned explicitly in the read-me and/or release note. Thanks for sharing your good work to the community!

skwasjer commented 1 year ago

I understand your point and will add a note somewhere.

Certain housekeeping does not always warrant explicit mentioning, that is all. I was not trying to hide it :p

Also, it is really easy to add RU back in programmatically if you'd really want to due to the nature of the configurability of the IbanRegistry

tdltdl commented 1 year ago

I understand ;-) Looking at the bright side of things :

  1. you'll make your support for Ukraine visible
  2. if you mention that it dit not break anything, that should not annoy anyone
  3. if on top you say, if you need it really it is easy for you to add it bec my library is flexible, that is even better ;-)