#21323 closed Cleanup/optimization (fixed)
Allow migrations.Operation to control their output.
Reported by: | loic84 | Owned by: | |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently MigrationWriter
controls the output of migrations and to do so it renders the various operations with a generic format.
The problem is operations such as CreateModel
end up writing all the fields on a single line, which makes the migration file difficult to review.
I propose that Operations
should control their own output.
I had a go at this in https://github.com/django/django/pull/1681.
Change History (6)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 11 years ago
comment:3 by , 11 years ago
I've cleaned up the code a bit by introducing a MigrationFormatter
class, but ultimately there isn't so much we can do; presentational logic in Python is quite verbose. Also the way the serializer is implemented requires formatting as we go, fixing that would require a significant refactor.
If we move to a better template system with white spaces handling (Jinja2), CreateModel.serialize()
could be shrunk significantly.
IMO being able to review the output of CreateModel
is sufficiently critical to justify an imperfect CreateModel.serialize()
method.
comment:4 by , 11 years ago
I've reworked the code in a way that doesn't affect the Operation
classes anymore, the changes are now contained in migrations/writer.py
.
comment:5 by , 11 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Me and loic84 discussed this on IRC a while back; I'm not a fan of how this currently looks (it makes the operations very hard to read and is confusing logic with presentation in a way) but it's a nice idea in principle.