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

Refactor: Align Extension Methods with Project Structure and Remove Unused Usings #1457

Closed
wants to merge 1 commit into from

Conversation

nirzaf
Copy link

@nirzaf nirzaf commented Jun 12, 2024

Removal of Unused Using Statements: To improve code readability and maintainability, all unused using statements have been removed from the project files.
Reorganization of Extension Methods: The extension methods have been moved to the Sustainsys.Saml2.AspNetCore namespace. This change aligns the location of these methods with the overall project structure, making the codebase easier to navigate and understand.
These changes do not affect the functionality of the code but contribute to cleaner and more organized codebase.

…to align with project structure and Removed all unused using statements.
Copy link
Member

@AndersAbel AndersAbel left a comment

Choose a reason for hiding this comment

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

Thanks for the massive code cleanup work, please see the comments on the namespaces.

using Sustainsys.Saml2.Bindings;
using Sustainsys.Saml2.Serialization;


// By convention, the extension methods are placed in the Microsoft shared
// namespace to allow convenient access from intellisense.
namespace Microsoft.Extensions.DependencyInjection;
Copy link
Member

@AndersAbel AndersAbel Jun 12, 2024

Choose a reason for hiding this comment

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

The AddSaml2() methods should be in the Microsoft.Extensions.DependcyInjection namespace. This aligns with how the Microsoft supplied handlers work - they put their Add* methods in that namespace even though the rest of the code is in Microsoft.AspNetCore.Authentication.*


namespace Microsoft.Extensions.DependencyInjection;
namespace Sustainsys.Saml2.AspNetCore;
Copy link
Member

Choose a reason for hiding this comment

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

This one is correct, the PostConfigure should be in our namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants