Ticket #1996: Add documentation on models affected by signals #2151
Conversation
|
🥳 Successfully deployed to developer sandbox za. |
…ttps://github.com/cisagov/manage.get.gov into za/1996-add-documentation-on-models-affected-by-signals
|
🥳 Successfully deployed to developer sandbox za. |
|
🥳 Successfully deployed to developer sandbox za. |
1 similar comment
|
🥳 Successfully deployed to developer sandbox za. |
|
🥳 Successfully deployed to developer sandbox za. |
zandercymatics
requested review from
therealslimhsiehdy,
CocoByte,
rachidatecs,
dave-kennedy-ecs and
erinysong
|
🥳 Successfully deployed to developer sandbox za. |
zandercymatics
changed the title
docs/developer/README.md
Outdated
| 2. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. | ||
| 3. Minimize logic complexity within the signal as much as possible. | ||
| 4. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. |
There was a problem hiding this comment.
I would move this to number one
There was a problem hiding this comment.
Great callout
There was a problem hiding this comment.
Approve but maybe wait on one more person to read really fast to make sure it makes sense to more devs as we want this to be readable and usable by future developers. @dave-kennedy-ecs I think it was you and Rachid once had a heck of time trouble shooting signals, maybe you would be a good candidate to at least give the readme file a quick look.
|
🥳 Successfully deployed to developer sandbox za. |
1 similar comment
|
🥳 Successfully deployed to developer sandbox za. |
zandercymatics
deleted the
za/add-documentation-on-models-affected-by-signals
branch
Ticket
Resolves #1996
Changes
Context for reviewers
In an eng huddle, we concluded that we should be more thorough in regards to our signals as they are often hard to debug. This PR adds some documentation as well as overarching guidelines which new and old developers can use when approaching them.