Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing inheritance of AccountAuthenticationProvider from vscode's AuthenticationProvider #3574

Open
sergeibbb opened this issue Sep 13, 2024 · 0 comments
Labels
feature New feature or request triage Needs to be looked at

Comments

@sergeibbb
Copy link
Member

AccountAuthenticationProvider has had inheritance from AuthenticationProvider of VSCode for being registered in VSCode as an account provider.

In #3524 we have stopped registering our provider in VSCode, so technically we the inheritance is not necessary anymore. So we can consider removal, but also we might decide to leave it as is.

I suppose the decision depends on the following:

  1. On the one hand, AccountAuthenticationProvider in its current state may contain methods that are implemented only to satisfy inheritance of AuthenticationProvider but are not actually in use anymore.
  2. On the other hand, the parent class can contain some methods that are in use and that will need to be re-implemented.

Therefore we need to weigh whether it's worth or not to remove the inheritance and implement. We can make the decision by:

  • just starting the replacement and seeing how it goes
  • analising the current code
  • however, maybe Eric or Keith or Ramin can say it immediately without even touching the code

So, if we decide to rid of the inheritance it should be done in this ticket. If we decide to leave the inheritance, this ticket can be just closed as "won't do"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request triage Needs to be looked at
Projects
None yet
Development

No branches or pull requests

1 participant