Skip to content

Conversation

Pirols
Copy link
Contributor

@Pirols Pirols commented Aug 22, 2025

TODO

@robodoo
Copy link
Contributor

robodoo commented Aug 22, 2025

Pull request status dashboard

@Pirols
Copy link
Contributor Author

Pirols commented Aug 22, 2025

@KangOl @aj-fuentes I'd like to receive some early feedback on the design choices before committing further to them.

@Pirols Pirols force-pushed the master-convert_m2o_to_m2m-pied branch from c4cc604 to f1a99cf Compare August 22, 2025 16:15
_validate_model(origin_model)

if not table_exists(cr, m2m_field):
raise SleepyDeveloperError("m2m table should already exist warning")
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this util should create the m2m the table (auto key?).

Still I don't get what you are checking here. Why should there be a table with the name of the m2m_field? Maybe you wanted to check a column?

If we auto-create the m2m table, we will allow simpler code for fields that are introduced as m2o and later changed to m2m in the same major version --think 18.2 and 18.4 for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. I did not want to duplicate the logic in create_m2m and/or provide the same kind of arguments here. But it is necessary. I guess I can close this in favor of #306

Copy link
Contributor

@aj-fuentes aj-fuentes Aug 25, 2025

Choose a reason for hiding this comment

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

I think you can just move your commit there... not sure if that's what you intend to do :)

EDIT (by Pirols): Communication hiccup solved in pm: this PR is not #304 and #306 is not #305.

@Pirols
Copy link
Contributor Author

Pirols commented Aug 25, 2025

In favor of #306

@Pirols Pirols closed this Aug 25, 2025
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.

3 participants