search

sonata-project / GoogleAuthenticator

[Abandoned] Library to integrate Google Authenticator into a PHP project
https://github.com/sonata-project/GoogleAuthenticator
MIT License
436 stars 120 forks source link

Remove final from GoogleAuthenticator class #110

Closed umpirsky closed 6 years ago

umpirsky commented 6 years ago

Final blocks 2.0 upgrade for https://github.com/scheb/two-factor-bundle/issues/147.

Will you consider making it possible to extend?

kunicmarko20 commented 6 years ago

I'm intentionally using version 1.1 of the Sonata bundle because for whatever reason the GoogleAuthenticator class is final in the 2.0 version, which makes it impossible to mock it in test cases.

This is not the reason for us to remove the final and there are other ways to write tests. We use final so people won't extend the classes which are not meant to be extended and make us unable to do changes to classes without a BC Breaks because there is someone that extended that class.

Still, that class should have an interface, and that would solve your "problem" with tests?

~Not sure if we can introduce an interface without it being a BC Break~, cc @greg0ire wdyt?

Since the class is final we probably can just add an interface and release a minor version with it.

umpirsky commented 6 years ago

That's what I assumed, thanks for confirmation.

If final answer is no, just close the issue.

Хвала!

greg0ire commented 6 years ago

I'm OK with adding an interface 👍

OskarStark commented 6 years ago

Me, too

@umpirsky are you willing to create a PR?

umpirsky commented 6 years ago

Done https://github.com/sonata-project/GoogleAuthenticator/pull/111.