Opened 4 years ago

Closed 11 months ago

Last modified 11 months 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 Changed 4 years ago by Mariusz Felisiak

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 Changed 3 years ago by Caio Ariede

Owner: changed from nobody to Caio Ariede
Status: newassigned

comment:3 Changed 3 years ago by Caio Ariede

Has patch: set

comment:4 Changed 3 years ago by Mariusz Felisiak

Patch needs improvement: set

comment:5 Changed 3 years ago by Jacob Walls

Patch needs improvement: unset

Author appears ready for another review.

comment:6 Changed 3 years ago by Carlton Gibson

Patch needs improvement: set

comment:7 Changed 21 months ago by Mariusz Felisiak

Owner: Caio Ariede deleted
Status: assignednew

comment:8 Changed 19 months ago by Sarah Abderemane

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 Changed 12 months ago by Sarah Abderemane

Owner: Sarah Abderemane deleted
Status: assignednew

comment:10 Changed 11 months ago by Joseph V Zammit

Owner: set to Joseph V Zammit
Status: newassigned

comment:11 Changed 11 months ago by Joseph V Zammit

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

comment:12 Changed 11 months ago by Carlton Gibson

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:13 Changed 11 months ago by Carlton Gibson <carlton.gibson@…>

Resolution: fixed
Status: assignedclosed

In 71e9694:

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

comment:14 Changed 11 months ago by Carlton Gibson <carlton.gibson@…>

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