You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Oleksii called me out hard 😄 for having an interface that will lead to unnecessary breaking changes. While I doubt many, if any, will create their own implementation of the IPasswordlessClient it is technically a breaking change to add a method to a public interface while it is not to add a new method to PasswordlessClient.
While I do like having interfaces for the purposes of mocking, the better more resilient thing to do is to make your own interface that you own in your application that wraps the usage you have of any said library. Bitwarden server follows this practice for our uses of Stripe with StripeAdapter. We should recommend users of our library, who want to mock our service, do the same.
What we can do today:
Change examples and documentation that use IPasswordlessClient to PasswordlessClient
Stop adding new methods to IPasswordlessClient and only add the implementation in PasswordlessClient.
When we do other and only when we do other breaking changes we can but don't have to:
Make IPasswordlessClient have the [EditorBrowsable(Never)] attribute.
We could do this before a breaking change without actually breaking anyone but users who had previous uses of IPasswordlessClient and want to use it elsewhere could be thrown off.
Change the AddHttpClient method use PasswordlessClient as service instead of IPasswordlessClient
This changes logs from looking like: System.Net.Http.HttpClient.IPasswordlessClient.ClientHandler to System.Net.Http.HttpClient.PasswordlessClient.ClientHandler this is breaking because some users could use that category name to tune logs. Rare, but could cause some annoyance to consumers.
This doesn't need to be done other than people being slightly confused why the interface name is showing up in the logs when they don't use the interface at all.
This also changes the name of the client for if people are manually configuring/injecting the HttpClient themselves, this is not a situation we support or document anywhere but it's something that can be done before that would no longer work.
Write a recommended migration path of just removing the I from the type.
(NOT RECOMMENDED) we could also take a more aggressive approach and add [Obsolete] to the type but I don't think this is warranted.
The text was updated successfully, but these errors were encountered:
justindbaur
changed the title
Depracate Autotelic Abstraction
Deprecate Autotelic Abstraction
Sep 9, 2023
Sorry, I didn't mean to call anyone out specifically 😅
As an interim solution, we can also add a remark to IPasswordlessClient to discourage the end-users using it for mocking/faking in tests and recommend creating a domain-local abstraction instead.
Oleksii called me out hard 😄 for having an interface that will lead to unnecessary breaking changes. While I doubt many, if any, will create their own implementation of the
IPasswordlessClient
it is technically a breaking change to add a method to a public interface while it is not to add a new method toPasswordlessClient
.While I do like having interfaces for the purposes of mocking, the better more resilient thing to do is to make your own interface that you own in your application that wraps the usage you have of any said library. Bitwarden server follows this practice for our uses of Stripe with
StripeAdapter
. We should recommend users of our library, who want to mock our service, do the same.What we can do today:
IPasswordlessClient
toPasswordlessClient
IPasswordlessClient
and only add the implementation inPasswordlessClient
.When we do other and only when we do other breaking changes we can but don't have to:
IPasswordlessClient
have the[EditorBrowsable(Never)]
attribute.IPasswordlessClient
and want to use it elsewhere could be thrown off.AddHttpClient
method usePasswordlessClient
as service instead ofIPasswordlessClient
System.Net.Http.HttpClient.IPasswordlessClient.ClientHandler
toSystem.Net.Http.HttpClient.PasswordlessClient.ClientHandler
this is breaking because some users could use that category name to tune logs. Rare, but could cause some annoyance to consumers.HttpClient
themselves, this is not a situation we support or document anywhere but it's something that can be done before that would no longer work.I
from the type.[Obsolete]
to the type but I don't think this is warranted.The text was updated successfully, but these errors were encountered: