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

Remove unnecessary options structs #390

Merged
merged 4 commits into from
Feb 24, 2023
Merged

Remove unnecessary options structs #390

merged 4 commits into from
Feb 24, 2023

Conversation

chlowell
Copy link
Collaborator

Part of #380. This removes the options bags for methods and constructors from the public API. These types are useless in application code because they aren't input to any function (applications set options by calling functions like WithAuthority() instead). They could conceivably be used in unusual test scenarios as part of a fake MSAL however, because these types distract from the real options API, I think we shouldn't export them before seeing a clear need for them; doing so would be non-breaking.

}
}

// Client is a representation of authentication client for public applications as defined in the
// package doc. For more information, visit https://docs.microsoft.com/azure/active-directory/develop/msal-client-applications.
type Client struct {
base base.Client
accessor cache.ExportReplace
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of these are in common with confidential client.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are ways we could share these fields but I haven't thought of one that's really better than duplicating them. We could for example move the common fields onto a new type and have constructor options set them there. However, accommodating options not shared between client types would then be difficult and more complex than duplicating the common fields.

@sonarcloud
Copy link

sonarcloud bot commented Feb 24, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
25.7% 25.7% Duplication

@chlowell chlowell merged commit cb956a2 into dev Feb 24, 2023
@chlowell chlowell deleted the chlowell/unexport-options branch February 24, 2023 19:14
@rayluo rayluo mentioned this pull request Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants