Add migration for Django 4.0. Closes #179

[atodorov]

@claudep I'm not sure why Django 4.0.1 created the new migration file but I'd appreciate it if you can merge this PR and push a new version to PyPI.

[claudep]

And what about people running Django<4? They would obtain a similar reverse migration after applying this one.

As per https://docs.djangoproject.com/en/stable/topics/migrations/#supporting-multiple-django-versions, we should wait for Django 4 being the minimal supported version before making this change.

[atodorov]

And what about people running Django<4? They would obtain a similar reverse migration after applying this one.

I still can't figure out which change in Django or django-contrib-comments what necessitates this migration. Maybe you can point me in the right direction.

What comes as a solution is to make the actual migration code version dependent and make it a noop operation if using Django < 4.0. How does that sound ?

[shaib]

@claudep I think there will be no back-migrations detected, because... @atodorov the change is probably this one https://docs.djangoproject.com/en/4.0/releases/4.0/#migrations-autodetector-changes

[claudep]

Thanks Shai for spotting the issue. Then I think it would be fine to just alter the affected related_name values in the existing migrations, isn't it?

[atodorov]

Then I think it would be fine to just alter the affected related_name values in the existing migrations, isn't it?

This isn't going to apply the change for existing installations, only in situations where you will install from scratch.

[claudep]

If there are noop migrations, like Shai suggests, we don't need for them being applied.

[atodorov]

If there are noop migrations, like Shai suggests, we don't need for them being applied.

I don't follow here. @shaib says there will be no reverse migrations but we have a forwards migration. I fail to see how modifying the existing migration sources makes a difference.

On an existing installation Django will not look at the previous migration files b/c they have already been applied and if you rely on makemigrations it will still produce a new migration (e.g. 0005_local). If you don't commit that to source in future package versions we'll add other migrations which could break the dependency chain b/c they will chain off of 0004 which is in git, not from the 0005_local which I happened to create on my existing installation.

Migrations are supposed to not be modified once generated, aren't they ?

From my understanding whatever changed we make to the package should result in a new migration anyway.

[claudep]

On an existing installation Django will not look at the previous migration files b/c they have already been applied and if you rely on makemigrations it will still produce a new migration (e.g. 0005_local).

AFAIK that's not true, Django is using previous migration files to detect new migrations to create.

Migrations are supposed to not be modified once generated, aren't they ?

Generally not, unless you are certain that migrations do not impact the database nor other migrations, which seems to be the case here.

[atodorov]

@claudep, @shaib I think my understanding of the root cause and the effects of a possible patch is still lacking some bits.

I can hack-up whatever you tell me to and push it as a PR but at this point I can be confident in the changes only after I try the new version against a copy of my production instance.

Let me know if you want me to follow up on this PR and how to modify it or close it if you are planning to do so yourselves.

[claudep]

@atodorov this would be my approach: #181 Would you be able to test it with your use case?

[claudep]

Replaced by #181