[Django] #32900: Migrations questioner uses bad grammar
Django
------------------------------------------------+------------------------
Reporter: Christian Ullrich | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 3.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 interactive questioner, when it asks what to do about changes in
nullability or adding a NOT NULL field, provides multiple options. Some of
these speak ''to'' the user, some ''as'' the user.
One example of several:
{{{
You are trying to add a non-nullable field 'id' to mymodel without a
default; we can't do that (the database needs something to populate
existing rows).
Please select a fix:
1) Provide a one-off default now (will be set on all existing rows with a
null value for this column)
2) Quit, and let me add a default in models.py
}}}
In option 1, Django offers the user the choice of entering the default
value. Option 2 instead is the user telling Django what to do.
Each time I read this, I'm asking myself who the "me" in option 2 is. The
most egregious case is in django.db.migrations.ask_not_null_alteration():
"[...] let ''me'' handle existing rows [...] (e.g. because ''you'' added a
RunPython [...]". In this sentence, "me" and "you" are the same.
Fix: Either reword the "provide" options to say "Let me enter a one-off
default now", or the "let me" options to the same style as the "provide"
options. I am very much in favor of the latter because I think this "me,
the user" style is terrible.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Christian Ullrich | Owner: nobody
Cleanup/optimization |
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Christian Ullrich:
Old description:
> The interactive questioner, when it asks what to do about changes in
> nullability or adding a NOT NULL field, provides multiple options. Some
> of these speak ''to'' the user, some ''as'' the user.
>
> One example of several:
>
> {{{
> You are trying to add a non-nullable field 'id' to mymodel without a
> default; we can't do that (the database needs something to populate
> existing rows).
> Please select a fix:
> 1) Provide a one-off default now (will be set on all existing rows with
> a null value for this column)
> 2) Quit, and let me add a default in models.py
> }}}
>
> In option 1, Django offers the user the choice of entering the default
> value. Option 2 instead is the user telling Django what to do.
>
> Each time I read this, I'm asking myself who the "me" in option 2 is. The
> most egregious case is in django.db.migrations.ask_not_null_alteration():
> "[...] let ''me'' handle existing rows [...] (e.g. because ''you'' added
> a RunPython [...]". In this sentence, "me" and "you" are the same.
>
> Fix: Either reword the "provide" options to say "Let me enter a one-off
> default now", or the "let me" options to the same style as the "provide"
> options. I am very much in favor of the latter because I think this "me,
> the user" style is terrible.
New description:
The interactive questioner, when it asks what to do about changes in
nullability or adding a NOT NULL field, provides multiple options. Some of
these speak ''to'' the user, some ''as'' the user.
One example of several:
{{{
You are trying to add a non-nullable field 'id' to mymodel without a
default; we can't do that (the database needs something to populate
existing rows).
Please select a fix:
1) Provide a one-off default now (will be set on all existing rows with a
null value for this column)
2) Quit, and let me add a default in models.py
}}}
In option 1, Django offers the user the choice of entering the default
value. Option 2 instead is the user telling Django what to do.
Each time I read this, I'm asking myself who the "me" in option 2 is. The
most egregious case is in
InteractiveMigrationQuestioner.ask_not_null_alteration(): "[...] let
''me'' handle existing rows [...] (e.g. because ''you'' added a RunPython
[...]". In this sentence, "me" and "you" are the same.
Fix: Either reword the "provide" options to say "Let me enter a one-off
default now", or the "let me" options to the same style as the "provide"
options. I am very much in favor of the latter because I think this "me,
the user" style is terrible.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:1>
Django
Reporter: Christian Ullrich | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Jacob Walls):
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Thanks, this has also occurred to me. For the example you quoted in full,
perhaps: "Quit, so that a default in models.py can be added later." And
something similarly passive for the other cases, avoiding the use of "you"
entirely.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:2>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mateo Radman):
* owner: nobody => Mateo Radman
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:3>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mateo Radman):
* status: assigned => closed
* resolution: => fixed
Comment:
[https://github.com/django/django/pull/14607 PR] is ready. Let me know if
you have any suggestions regarding phrasing the questions.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:4>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: closed => new
* has_patch: 0 => 1
* resolution: fixed =>
Comment:
When adding a PR, the correct action is to check "Has patch" rather than
close the ticket. See
[https://docs.djangoproject.com/en/dev/internals/contributing/triaging-
tickets/ Triaging tickets] and the "According to the ticket's flags, the
next step(s) to move this issue forward are:" section of each open ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:5>
Django
Reporter: Christian Ullrich | Owner: Muhammad
Type: | Hammad
Cleanup/optimization | Status: assigned
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Mateo Radman => Muhammad Hammad
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:6>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: Muhammad Hammad => Mateo Radman
Comment:
Muhammad, this ticket is already assigned to Mateo. Please try to find a
ticket that is not assigned.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:7>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mateo Radman):
I think [https://github.com/django/django/pull/14607 PR] is ready to be
merged. Thanks for everyone’s suggestions and involvement.
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:8>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:9>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:10>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:11>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:12>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:13>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d00fb4d2d6f375ace2b4fe35a4acde8e476000d2" d00fb4d2]:
{{{
#!CommitTicketReference repository=""
revision="d00fb4d2d6f375ace2b4fe35a4acde8e476000d2"
Refs #32900 -- Added test for ignoring the default value in
InteractiveMigrationQuestioner.ask_not_null_alteration().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:14>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"02bc7161ec477afd4a7b328936eb8adac078d7b9" 02bc7161]:
{{{
#!CommitTicketReference repository=""
revision="02bc7161ec477afd4a7b328936eb8adac078d7b9"
Fixed #32900 -- Improved migrations questioner prompts.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:16>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"61c5eae516257d4827b8a12da2b714f05ac88a9f" 61c5eae]:
{{{
#!CommitTicketReference repository=""
revision="61c5eae516257d4827b8a12da2b714f05ac88a9f"
Refs #32900 -- Added makemigrations tests for messages in interactive
mode.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:15>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Component: Migrations | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"aa0d796e37c4b8056148de2f68726aae9d20399c" aa0d796e]:
{{{
#!CommitTicketReference repository=""
revision="aa0d796e37c4b8056148de2f68726aae9d20399c"
Refs #32900 -- Restored '[y/N]' in questioner prompt when merging
migrations.
Regression in 02bc7161ec477afd4a7b328936eb8adac078d7b9.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:17>
Django
Reporter: Christian Ullrich | Owner: Mateo
Type: | Radman
Cleanup/optimization | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"1aada25eeb8debabe5e1a50d64843b085a95237b" 1aada25e]:
{{{
#!CommitTicketReference repository=""
revision="1aada25eeb8debabe5e1a50d64843b085a95237b"
[4.0.x] Refs #32900 -- Restored '[y/N]' in questioner prompt when merging
migrations.
Regression in 02bc7161ec477afd4a7b328936eb8adac078d7b9.
Backport of aa0d796e37c4b8056148de2f68726aae9d20399c from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32900#comment:18>