Stop automatically creating migrations on certain fields

[ericholscher]

Currently we get a bunch of junk migrations when we rename help_text or choices in a model. I don't think these are actually useful, and we should find a way to stop them.

There is a suggestion presented here: https://stackoverflow.com/a/39801321 -- it basically monkey-patches the Django migrations to ignore these fields. I'm not in love with this solution, but I also think we should find a way to fix this, as it causes a lot of issues with new contributors.

[agjohnson]

I'd say I'm -1 on this probably. What about a CI check that does something similar to:

./manage.py makemigrations --check --dry-run

(We don't have a django version that supports this yet)

Similar: https://gist.github.com/nealtodd/a8f87b0d95e73eb482c5

A simple test case could error out when a migration is required.

[ericholscher]

Thats good and all, but it still leaves a tangled mess of migration filenames and PR's. We end up with a bunch of PR's with migrations that are out of order, and that require additional work to clean up, with no real value as a migration since they don't effect DB state.

What is the value of having a bunch of migrations that don't actually do anything to the DB state?

[humitos]

I'd be against of hacking the makemigrations command because we could be breaking things without noticing them immediately but, reading the answer from Andrew here (https://code.djangoproject.com/ticket/21498#comment:6) and the whole discussion, it seems that it could affect some weird usages of those fields so I think we will be safe on ignoring those fields.

I'd say that we want to have both things:

• a check in the CI
• do not generate migrations files for non-db fields

Since it seems there is not a better solution for this, I'd go over the monkey patch one which we could improve later if we find a better one.

[agjohnson]

I think this is 2 phase. Fix the current errors, then use CI to ensure that any new changes that should incur a migration make a corresponding migration. The fact that we have missing migrations now is a secondary issue -- the main issue is that users don't know they need to create these when making changes, linting. Out of order/etc migrations would all be picked up in review.

Hacking makemigrations sounds like a scary, maybe breaking step when compared to just CI failure.

[xrmx]

I understand the annoyance when changing content of text but some of these are from different types which are worth to fix imho.

[agjohnson]

This is going stale, so we should either accept or close it probably. I'm -1 on monkeypatching, as i think it will give us different problems or inconsistencies. I hesitate getting too creative with anything that touches out db at such a low level.

We can easily add the missing migrations, but to automate for future consistency, i think a simple CI check to fail PRs without necessary migraitons solves the problem we have.

[stsewd]

+1 on CI check

[stsewd]

We have this migrations to apply before we can do something, the migrations for integrations are the trickiest

Migrations for 'builds':
  readthedocs/builds/migrations/0005_auto_20181025_1710.py
    - Alter field error on build
    - Alter field output on build
    - Alter field state on build
    - Alter field type on build
    - Alter field privacy_level on version
    - Alter field slug on version
    - Alter field type on version
    - Alter field from_slug on versionalias
    - Alter field to_slug on versionalias
Migrations for 'gold':
  readthedocs/gold/migrations/0003_auto_20181025_1710.py
    - Alter field level on golduser
Migrations for 'integrations':
  readthedocs/integrations/migrations/0003_auto_20181025_1710.py
    - Create proxy model BitbucketWebhook
    - Create proxy model GenericAPIWebhook
    - Create proxy model GitHubWebhook
    - Create proxy model GitLabWebhook
    - Alter field integration_type on integration
Migrations for 'oauth':
  readthedocs/oauth/migrations/0009_auto_20181025_1710.py
    - Alter field clone_url on remoterepository
    - Alter field ssh_url on remoterepository
    - Alter field vcs on remoterepository
Migrations for 'projects':
  readthedocs/projects/migrations/0028_auto_20181025_1710.py
    - Alter field https on domain
    - Alter field comment_moderation on project
    - Alter field documentation_type on project
    - Alter field language on project
    - Alter field privacy_level on project
    - Alter field python_interpreter on project
    - Alter field version_privacy_level on project
Migrations for 'redirects':
  readthedocs/redirects/migrations/0002_auto_20181025_1710.py
    - Alter field redirect_type on redirect
    - Alter field to_url on redirect

[ericholscher]

@stsewd what is tricky about the integrations? Looks like it's just adding options?

[stsewd]

@ericholscher the integrations app is creating models

[ericholscher]

Just proxy models, no?

[stsewd]

Yeah, looks like so