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

Disable auto registration for base classes #218

Closed
wants to merge 2 commits into from

Conversation

cllns
Copy link
Member

@cllns cllns commented Jul 27, 2024

Cleanup for #180 and #197.

We don't want the base classes to be auto_registered in the app's container, since they're meant to be used as parent classes, not used as instances directly.

@timriley
Copy link
Member

Thanks for being conscientious and looping back to this, @cllns!

However, I think we're already in the clear here: all of the files you touch here are generated into db/ directories, and since db is in our default no_auto_register_paths, nothing in this directory will be auto-registered anyway.

And I don't think we should put these directives on files that are already excluded courtesy of their location, especially if it's not likely those files will ever move anywhere, since the unnecessary inclusion of the directive may confuse our users.

So I think we can close this one 😄

@cllns
Copy link
Member Author

cllns commented Jul 29, 2024

Oh sweet, I missed that! Thanks

@cllns cllns closed this Jul 29, 2024
@timriley timriley deleted the disable-auto-registration-for-base-classes branch July 29, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants