Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create kpi-backend-development.md #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

bufke
Copy link
Contributor

@bufke bufke commented Feb 2, 2023

Attempting to document kpi backend, like we have for frontend.

Attempting to document kpi backend, like we have for frontend.
@jnm jnm self-requested a review February 2, 2023 19:28
@jnm jnm self-assigned this Feb 2, 2023
@@ -0,0 +1,29 @@
---
title: KPI Backend Development
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnm, pull from various sources (zulip, github issue, etc.) to flesh this out.
Probably remove the break early thing? It's likely more of a judgment call than a rule.
Add guidance for assertNumQueries(): set it to fail, read the queries printed out, understand them and ask questions if necessary, and if the queries then are okay, set the count. Investigate the use of ranges for these assertions. Discuss fixtures so that list-type queries return enough objects that bad query-per-each-result patterns are obvious.
Make sure your unit tests can fail!!!
etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnm should we merge this as is and try to improve it as we go?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jnm @bufke should we merge this as is? :D

do_the_thing()
```

- Avoid unnecessary Django migrations. New pull requestions should contain no more than 1 migration, unless necessary.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pull requestions" should be "pull requests"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants