django-test-migrations icon indicating copy to clipboard operation
django-test-migrations copied to clipboard

Create a check for initial migrations

Open sobolevn opened this issue 4 years ago • 6 comments

Today I found a code like this:

class Migration(migrations.Migration):

    # initial = True
    dependencies = []

And this was an initial migration for an app.

This suprised me:

  1. It works correctly
  2. It does not produce any warnings
  3. It is cryptic

What am I concerned about? I have an impression that all migrations with dependencies = [] must be initial, because otherwise they should be dependent on a previous migration. Am I correct?

So, I guess we can check this pretty easily in case initial really works this way.

sobolevn avatar Dec 24 '20 09:12 sobolevn

That's strange, however, according to django docs about Migration.initial, such migration can be valid initial migration:

Initial migrations are marked with an initial = True class attribute on the migration class. If an initial class attribute isn’t found, a migration will be considered “initial” if it is the first migration in the app (i.e. if it has no dependencies on any other migration in the same app).

I agree that this is really confusing and IMHO it's better to be explicit and set initial = True, so adding a check for this will be really nice.

What do you think about creating a new app (something like django_test_migrations.contrib.django_checks.MigrationClasses) for this check (and potentially a few others in the future)?

skarzi avatar Dec 24 '20 10:12 skarzi

@skarzi absolutely great idea! 👍

Do you have any other checks in mind?

sobolevn avatar Dec 24 '20 11:12 sobolevn

I was also thinking about adding this feature to django-migration-linter, but after reading its short description in README.md:

Detect backward incompatible migrations for your Django project. It will save you time by making sure migrations will not break with a older codebase.

I am not sure it's a proper place for such thing

skarzi avatar Dec 24 '20 11:12 skarzi

Yeah, I also consider django-migration-linter as django-incompatible-migration-linter 🙂

sobolevn avatar Dec 24 '20 11:12 sobolevn

Hi

Is there any activity here? Is it still relevant?

And if so, what do you expect from this feature?

denisSurkov avatar Jan 14 '22 18:01 denisSurkov

hello @denisSurkov :wave:

I think it's still relevant, however, we haven't got time to work on this issue, so any help is really appreciated. To be honest, I haven't checked how to implement this feature, but the general process could look like this:

  1. Let's add a new file with checks to this directory a) Maybe MigrationLoader.graph.root_nodes() can be used to implement initial = True check
  2. Let's add a new app config class MigrationClasses to this file so migration-related checks can be included separately

Of course, don't forget about unit tests of all newly introduced code. Let's ping me if you have any questions etc ;)

skarzi avatar Jan 17 '22 09:01 skarzi