django
django copied to clipboard
Fixed #25068 -- Adding migrations support for models with metaclasses.
Please test locally instead of running the tests via Jenkins. 👍
Please test locally instead of running the tests via Jenkins. 👍
@smithdc1 Sorry, how do you do that? I'm very new to this
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! 🤞
@felixxm everything should be ready now for another review
@felixxm have you had another chance to review this PR yet?
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
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.
Anything I can do to help get this into the next release?
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!
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 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?
@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.
Closing due to inactivity.