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

Simplify ITenantInfo? #836

Open
fdcastel opened this issue Jun 3, 2024 · 2 comments
Open

Simplify ITenantInfo? #836

fdcastel opened this issue Jun 3, 2024 · 2 comments

Comments

@fdcastel
Copy link

fdcastel commented Jun 3, 2024

  1. Why does ITenantInfo have property setters?

    To the best of my understanding, they are only used in tests.

    ITenantInfo interface could be simplified to use only getters. And tests may use a TestTenantInfo with setters.

  2. While on this page, why Identifier and Name?

    The absolutely minimum required is Id. And it should not be nullable.

    Other properties seem to be used only for some specific cases/strategies. It could be refactored.

Sadly, the 7.0 release has already shipped. Maybe in 8.0?

I can work on some PRs, if interested.

@fdcastel
Copy link
Author

fdcastel commented Jun 3, 2024

Update:

To the best of my understanding, they are only used in tests.

Correction: The interface setters are not used anywhere. Tests always use TenantInfo class, which already does have setters.

ITenantInfo interface can be simplified to use only getters.

I did a POC right now in a local fork and everything builds fine.
I can provide my own custom ITenantInfo implementation without the need for setters.

@AndrewTriesToCode
Copy link
Sponsor Contributor

AndrewTriesToCode commented Jun 5, 2024

Yes you are right. I would like to clean this up on the next major release and have ITenantInfo instances be treated more like an immutable.

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

No branches or pull requests

2 participants