[Django] #34974: makemigrations --check does not show diff as documented

26 views
Skip to first unread message

Django

unread,
Nov 17, 2023, 8:14:33 AM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-----------------------------------------+------------------------
Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 4.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
The [docs](https://docs.djangoproject.com/en/4.2/ref/django-admin/#django-
admin-makemigrations) say:

>--check¶
>Makes makemigrations exit with a non-zero status when model changes
without migrations are detected. **Implies --dry-run**.

which:
>--dry-run¶
>**Shows what migrations would be made** without actually writing any
migrations files to disk. Using this option along with --verbosity 3 will
also show the complete migrations files that would be written.

emphasis mine.

In CI, would be very useful to have a single command which both renders
the missing/problematic migration and also exits non-zero; I think this
should be `makemigrations --check`, as it's already documented as doing.

Alternatively, maybe `--check` should _not_ imply `--dry-run` at all in
any sense - i.e. it should write to disk if `--dry-run` not given. I'm not
sure I like that though, I think it makes sense that 'check' is
idempotent.

Black for example has `--diff` rather than `--dry-run`, and both prevent
writing to disk, but can be combined for an exit code as well as the
change printed:

>--check Don't write the files back, just return
the
status. Return code 0 means nothing
would
change. Return code 1 means some files
would
be reformatted. Return code 123 means
there
was an internal error.

>--diff Don't write the files back, just output a
diff for each file on stdout.

(can't figure out this formatting...)

--
Ticket URL: <https://code.djangoproject.com/ticket/34974>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 17, 2023, 8:21:36 AM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-------------------------------+--------------------------------------

Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: closed
Component: Uncategorized | Version: 4.2
Severity: Normal | Resolution: duplicate

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by David Sanders):

* status: new => closed
* resolution: => duplicate


Comment:

Hi Oliver,

Thanks for the report. The documentation was for a patch that was recently
added to the 4.2.x branch with #34457. The patch was only merged 8 days
ago and doesn't look like it's released yet. Please bear with us as we
await its release :)

Marking as duplicate of #34457.

--
Ticket URL: <https://code.djangoproject.com/ticket/34974#comment:1>

Django

unread,
Nov 17, 2023, 8:48:50 AM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-------------------------------+--------------------------------------
Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: closed
Component: Uncategorized | Version: 4.2
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by David Sanders):

Also FYI here's the release post for 4.2.8:
https://docs.djangoproject.com/en/dev/releases/4.2.8/

Expected 4 December 2023

--
Ticket URL: <https://code.djangoproject.com/ticket/34974#comment:2>

Django

unread,
Nov 17, 2023, 9:06:43 AM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-------------------------------+--------------------------------------
Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: closed
Component: Uncategorized | Version: 4.2
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Oliver Ford):

Ah, what I was actually hoping for was the full contents of the file, not
just the would-be file name, and model it affects.

But it seems I misunderstood, and `--dry-run` doesn't do that either. Is
there already a feature request for a `--dry-run --diff` or something do
you know?

i.e. to build in what would be worked around by _not_ using `--dry-run`,
grepping out the filename and printing the contents.

--
Ticket URL: <https://code.djangoproject.com/ticket/34974#comment:3>

Django

unread,
Nov 17, 2023, 9:21:31 AM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-------------------------------+--------------------------------------
Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: closed
Component: Uncategorized | Version: 4.2
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by David Sanders):

> Is there already a feature request for a --dry-run --diff or something
do you know?

I don't believe there is sorry 🤷‍♂️

If you want to write something yourself you can update makemigrations to
simply write to `sys.stdout`:

{{{
@@ -360,9 +360,8 @@ class Command(BaseCommand):
# We just do this once per app
directory_created[app_label] = True
migration_string = writer.as_string()
- with open(writer.path, "w", encoding="utf-8") as fh:
- fh.write(migration_string)
- self.written_files.append(writer.path)
+ sys.stdout.write(migration_string)
+ self.written_files.append(writer.path)
if update_previous_migration_paths:
prev_path =
update_previous_migration_paths[app_label]
rel_prev_path = self.get_relative_path(prev_path)
}}}

That will print both the log message that shows the written file + the
contents. You can disable the logging easily with `-v 0` 👍

--
Ticket URL: <https://code.djangoproject.com/ticket/34974#comment:4>

Django

unread,
Nov 17, 2023, 9:04:13 PM11/17/23
to django-...@googlegroups.com
#34974: makemigrations --check does not show diff as documented
-------------------------------+--------------------------------------
Reporter: Oliver Ford | Owner: nobody
Type: Uncategorized | Status: closed
Component: Uncategorized | Version: 4.2
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------

Comment (by Jacob Walls):

There is a simple way to get the full migration file: you can run
`makemigrations --check --verbosity 3` (that is, once 4.2.8 is out).

--
Ticket URL: <https://code.djangoproject.com/ticket/34974#comment:5>

Reply all
Reply to author
Forward
0 new messages