Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a check for initial migrations #161

Open
sobolevn opened this issue Dec 24, 2020 · 6 comments
Open

Create a check for initial migrations #161

sobolevn opened this issue Dec 24, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@sobolevn
Copy link
Member

sobolevn commented Dec 24, 2020

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 sobolevn added the enhancement New feature or request label Dec 24, 2020
@skarzi
Copy link
Collaborator

skarzi commented Dec 24, 2020

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)?

@sobolevn
Copy link
Member Author

@skarzi absolutely great idea! 👍

Do you have any other checks in mind?

@skarzi
Copy link
Collaborator

skarzi commented Dec 24, 2020

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

@sobolevn
Copy link
Member Author

sobolevn commented Dec 24, 2020

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

@denisSurkov
Copy link

Hi

Is there any activity here? Is it still relevant?

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

@skarzi
Copy link
Collaborator

skarzi commented Jan 17, 2022

hello @denisSurkov 👋

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants