django icon indicating copy to clipboard operation
django copied to clipboard

Fixed #25068 -- Adding migrations support for models with metaclasses.

Open ethanhowell opened this issue 3 years ago • 9 comments

Ticket 25068

Fix bug that doesn't allow models with multiple bases to be migrated

ethanhowell avatar Sep 03 '20 19:09 ethanhowell

Please test locally instead of running the tests via Jenkins. 👍

smithdc1 avatar Sep 03 '20 20:09 smithdc1

Please test locally instead of running the tests via Jenkins. 👍

@smithdc1 Sorry, how do you do that? I'm very new to this

ethanhowell avatar Sep 03 '20 20:09 ethanhowell

There are some notes in the contributing guide, this page here is most relevant.

https://docs.djangoproject.com/en/3.1/internals/contributing/writing-code/unit-tests/#quickstart

In summary - the quick start guide gets you setup (install requirements) and shows how to run the entire test suite. This is likely to take some time so it is likely you only want to run part of the test suite.

Say you only want to run the migrations tests you can run python runtests.py migrations

Good luck! 🤞

smithdc1 avatar Sep 03 '20 21:09 smithdc1

@felixxm everything should be ready now for another review

ethanhowell avatar Sep 22 '20 21:09 ethanhowell

@felixxm have you had another chance to review this PR yet?

ethanhowell avatar Oct 07 '20 22:10 ethanhowell

Hi @ethanhowell, I can give you a tip for moving this along -- uncheck Needs Tests and Needs Improvement on the underlying ticket to get back in the review queue so long as you've dealt with the feedback so far. Cheers

jacobtylerwalls avatar Oct 25 '20 17:10 jacobtylerwalls

Until this makes it to a release, I'm unable to make migrations automatically. I guess not many people use multiple meta classes with their models.

pwhipp avatar Aug 31 '21 00:08 pwhipp

Anything I can do to help get this into the next release?

pwhipp avatar Mar 01 '22 23:03 pwhipp

Anything I can do to help get this into the next release?

@pwhipp All the conflicts have been resolved, so I think this just needs to be merged!

ethanhowell avatar Mar 10 '22 18:03 ethanhowell

Hi guys, what we need to do here in order to push this MR? The current state is that it's not possible to run migrations on models with metaclasses.

TomHaii avatar Mar 11 '24 09:03 TomHaii

@TomHaii Making comments like this doesn't help and it comes across a little rude.

The reason is given in a review above:

Unfortunately, the current solution doesn't track metaclass changes...

Following through to ticket-25068, I can also see that it is flagged as "patch needs improvement".

Because that flag is set, the ticket hasn't been put on the queue for review by the fellows again.

It is also unclear whether the issue raised has actually been addressed from subsequent comments. In fact, it probably hasn't:

This is a significant step forwards even if it fails to track metaclass changes.

If you want to see some movement here, perhaps you could take this forward and fix the problem identified?

ngnpope avatar Mar 11 '24 12:03 ngnpope

@TomHaii Making comments like this doesn't help and it comes across a little rude.

The reason is given in a review above:

Unfortunately, the current solution doesn't track metaclass changes...

Following through to ticket-25068, I can also see that it is flagged as "patch needs improvement".

Because that flag is set, the ticket hasn't been put on the queue for review by the fellows again.

It is also unclear whether the issue raised has actually been addressed from subsequent comments. In fact, it probably hasn't:

This is a significant step forwards even if it fails to track metaclass changes.

If you want to see some movement here, perhaps you could take this forward and fix the problem identified?

Edited my comment. Didn't meant it to be rude and sorry if it did. Thanks for the info, will research the current identified issues to see if I can contribute to this PR.

TomHaii avatar Mar 11 '24 12:03 TomHaii

Closing due to inactivity.

felixxm avatar Mar 12 '24 04:03 felixxm