Opened 5 years ago

Closed 2 years ago

Last modified 2 years ago

#30801 closed Cleanup/optimization (fixed)

Improve guidance for making good use of signals.

Reported by: Aymeric Augustin Owned by: Joseph V Zammit
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Adam Johnson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is a follow-up to https://github.com/django/django/pull/11814 and https://groups.google.com/d/msg/django-developers/c5sFZ5BEujI/jVLsfSYBAwAJ.

I have two suggestions to improve the documentation of signals.

  1. In ref/signals.txt, for each signal, document the alternatives, which will usually be more robust and maintainable:
  • Instead of a model signals, you can override the corresponding method in the model class — unless that model is provided by a library and cannot be swapped
  • Instead of request/response signals, you can provide a middleware.
  • As far as I can tell, other signals don't have a great alternative. You could replace connection_created by a custom database backend but that's a lot more work.
  1. In the introduction of topics/signals.txt, replace the list of signals with a good example. In my opinion, setting_changed is likely the best example: many pluggable apps need it in their test suite.

Change History (14)

comment:1 by Mariusz Felisiak, 5 years ago

Easy pickings: unset
Summary: Improve guidance for making good use of signalsImprove guidance for making good use of signals.
Triage Stage: UnreviewedAccepted

Thanks. I wouldn't say that providing good examples and alternatives is "Easy picking" that's why I unchecked this flag.

comment:2 by Caio Ariede, 4 years ago

Owner: changed from nobody to Caio Ariede
Status: newassigned

comment:3 by Caio Ariede, 4 years ago

Has patch: set

comment:4 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:5 by Jacob Walls, 4 years ago

Patch needs improvement: unset

Author appears ready for another review.

comment:6 by Carlton Gibson, 4 years ago

Patch needs improvement: set

comment:7 by Mariusz Felisiak, 3 years ago

Owner: Caio Ariede removed
Status: assignednew

comment:8 by Sarah Abderemane, 3 years ago

Owner: set to Sarah Abderemane
Status: newassigned

Hi there, I would like to give a try on this ticket, but I'm not sure to understand what I should do with this part :

As far as I can tell, other signals don't have a great alternative. You could replace connection_created by a custom database backend but that's a lot more work.

It's to redirect to the part of the documentation where we define custom database or something else?

comment:9 by Sarah Abderemane, 2 years ago

Owner: Sarah Abderemane removed
Status: assignednew

comment:10 by Joseph V Zammit, 2 years ago

Owner: set to Joseph V Zammit
Status: newassigned

comment:11 by Joseph V Zammit, 2 years ago

Tentative PR is open here: ​https://github.com/django/django/pull/16221

comment:12 by Carlton Gibson, 2 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:13 by Carlton Gibson <carlton.gibson@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In 71e9694:

Fixed #30801 -- Improved guidance for making good use of signals.

comment:14 by Carlton Gibson <carlton.gibson@…>, 2 years ago

In 018311d:

[4.1.x] Fixed #30801 -- Improved guidance for making good use of signals.

Backport of 71e9694856627d4027c9df066045f0e1c2b5cf27 from main

Note: See TracTickets for help on using tickets.
Back to Top