Something I’ve noticed a lot with PRs is that someone attempts to close a ticket, and when we see their implementation we jump on it with tons of requests and nits (Yes, I am on both ends of this).
To cut back on wasted developer work, tickets that are feature requests should have a section called Acceptance Criteria. This section should detail everything that needs to be present/absent/tested for the ticket to be closed. These could be very specific technical features or they could be broader UX or architectural statements. They could also detail tests necessary to ensure the new feature works correctly or not including edge cases.
Example
Feature Request
Description
We should update the password hashing mechanism to use foolib.
User Stories
I want to use a lib that handles multiple hashing algorithms so I don’t have to write boilerplate code.
Acceptance Criteria
There is a database migration that adds the new pw hash columns to the database
There are tests for users created only using the old pw hash mechanism to ensure they can still log in
Once a user has logged in using the new mechanism, the old pw_hash column is cleared
Excellent suggestion. I would propose allowing for “TBD” in that section of the template, since it can be a fair bit of work to refine / unpack these criteria. But I think it makes sense that it should be filled in before any development work begins.
I think this is a fine idea. I agree with @eloquence that we should allow TBD. I do also think we should be vigilant in encouraging one another to not ask for unrelated issues to be resolved during review of a PR and instead resolve in separate PRs. That said, it’s normal that sometimes issues become apparent only after some initial exploratory work has been done.
Yup. Agreed that TBD makes sense. In the .github template, there should be a note that the section is optional since non-devs or people who aren’t extraordinarily familiar with the project might not be able to meaningfully set acceptance criteria.
