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

[RFC][IMP] modules, models: remove inherits #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pirols
Copy link
Contributor

@Pirols Pirols commented Feb 28, 2025

When removing a module, all its models (and their fields) are also removed. However inherits of such models had to be cleaned up manually.

@Pirols Pirols requested review from KangOl and aj-fuentes February 28, 2025 18:02
@robodoo
Copy link
Contributor

robodoo commented Feb 28, 2025

Pull request status dashboard

@Pirols
Copy link
Contributor Author

Pirols commented Feb 28, 2025

@Pirols Pirols force-pushed the master-remove_inherits-pied branch 4 times, most recently from 64b2f1f to 9a2eb3e Compare February 28, 2025 18:43
@Pirols
Copy link
Contributor Author

Pirols commented Feb 28, 2025

upgradeci retry with always only account

@Pirols
Copy link
Contributor Author

Pirols commented Feb 28, 2025

In principle this should be the default behaviour. However that would break backwards compatibility with code that used to deal with the inherits after the removal of the module/model.
The flags preserving worse-legacy-behavior are not ideal then, but probably necessary without a versioning system. Takes?

Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

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

Since removing inherits may require, or have required, some extra steps, I believe that adding a new flag (False by default) is the best option.

When removing a module, all its models (and their fields) are also removed.
However inherits of such models had to be cleaned up manually.
@Pirols Pirols force-pushed the master-remove_inherits-pied branch from 9a2eb3e to 45921f4 Compare March 3, 2025 08:43
@KangOl
Copy link
Contributor

KangOl commented Mar 3, 2025

In order to catch all potential issues:
upgradeci retry with all modules in all versions

(that will take a while...)

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.

4 participants