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

Rename LocalAuthenticationOptions to LocalAuthentication #152

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

Widcket
Copy link
Collaborator

@Widcket Widcket commented Aug 25, 2022

⚠️ This PR contains breaking changes

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

📋 Changes

This PR renames the LocalAuthenticationOptions class to just LocalAuthentication. This class is used as follows:

const options =
    LocalAuthenticationOptions(title: 'Please authenticate to continue');
final auth0 = Auth0('YOUR_AUTH0_DOMAIN', 'YOUR_AUTH0_CLIENT_ID',
    localAuthentication: options);
final credentials = await auth0.credentialsManager.credentials();

Setting it will enable the local authentication feature, which is disabled by default. So it's actually modeling the local authentication feature itself, instead of simply holding settings for a feature that is already in use. As such, it should be called just LocalAuthentication, without the Options suffix.

Compare it to the IdTokenValidationConfig class, which is used as follows:

const config = IdTokenValidationConfig(leeway: 10);
final credentials =
    await auth0.webAuthentication().login(idTokenValidationConfig: config);

The ID token validation feature is always enabled. This class merely allows to configure it, so it has Config as part of the class name, and also as part of the parameter name. It is not modeling the feature itself.

Config and Options have pretty much the same meaning in this context, yet are being used in classes that have different roles and achieve different results, which is inconsistent and might be confusing.

@Widcket Widcket added the review:tiny Tiny review label Aug 25, 2022
@Widcket Widcket requested a review from a team as a code owner August 25, 2022 02:48
@Widcket Widcket merged commit db9765e into ga Aug 25, 2022
@Widcket Widcket deleted the fix/rename-localauthenticationoptions branch August 25, 2022 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:tiny Tiny review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants