Use keyword argument rather than a positional argument for on_delete in the ForeignKey.on_delete docs.

[kay]

The code snippet for ​`ForeignKey.on_delete` in the docs does not spell out on_delete, which looks odd and can be confusing.

While further up, ​at the start of the ForeignKey section, it is explained to be a positional argument, the subsection documenting the argument itself doesn't seem like the best place to demonstrate that it isn't actually a keyword argument. I'd therefore suggest to add it in.

For variety/to remind users of its positional nature, it could instead be removed from any of the preceding code blocks, which all include it. The ​`products/models.py` example might make sense as it's only the second example in the overall ForeignKey section but identical to the first example.

Relatedly, the intro to the ​Arguments section, which leads into on_delete, is ambiguously worded. It says, "ForeignKey accepts other arguments that define the details of how the relation works." (emphasis mine), suggesting what's next are optional arguments or ones not yet discussed... when on_delete is neither.

[Sarah Boyce]

Agreed. Would you like to provide a patch?

I would say this doesn't need a ticket but as you've raised one, accepting

[Tim Graham]

on_delete was an optional argument until Django 2.0 (#21127, ddd3268975dca9094d94ab1df56dae0a24a58865). At that time, the tests were updated to remove on_delete= (for brevity) and on_delete= was added to the documentation examples as a keyword argument (for backward compatibility with older Django versions).

I think some of the confusion comes from documentation that wasn't completely updated at that time.

Since on_delete has now been a required argument for years, I think there's no more value in advocating it be a keyword argument in model definitions.

[Claude Paroz]

In the same vein as we required all argument of form fields to be keyword arguments, I'd also be in favor of having mostly keyword arguments for model fields. In my personal practice, only the verbose_name is used as first positional argument.

[kay]

Replying to Tim Graham:

Since on_delete has now been a required argument for years, I think there's no more value in advocating it be a keyword argument in model definitions.

Replying to Claude Paroz:

In the same vein as we required all argument of form fields to be keyword arguments, I'd also be in favor of having mostly keyword arguments for model fields. In my personal practice, only the verbose_name is used as first positional argument.

Just to clarify: I wasn't looking to start a discussion about argument types on model fields, or to advocate one type over another. This is solely about the docs and code snippets/examples contained within them. Further discussions about implementation should probably go in a separate ticket.

Replying to Sarah Boyce:

Agreed. Would you like to provide a patch?

I would say this doesn't need a ticket but as you've raised one, accepting

Yeah, I gathered that the suggested change may be small and "insignificant" enough to not necessitate a ticket, but I didn't want to stuff a bunch of explanations in a PR and potentially have to back and forth there & generally thought it wise(r) to follow the proper workflow considering I haven't contributed before. (The CONTRIBUTING file on GitHub warns that "anything more than fixing a typo" is considered a non-trivial PR.)

Will get working on a patch!