Pull request and peer review guidelines, best practices and conventions.

[karltaylor]

As we grow pull requests and peer reviews are becoming essential part of our development process. Pull requests allow developers to share their changes with other team members and get feedback on their work, while peer reviews ensure that code is of high quality, follows best practices, and meets project requirements. However, to make the most out of these processes, it is important to follow certain best practices.

Some ideas from Chat GPT:

• Keep pull requests small and focused on a single feature or bug fix.
• Write clear and descriptive titles and descriptions for pull requests.
• Include tests with your code changes and make sure they pass before submitting a pull request.
• Review and address any code quality issues, such as code smells or unnecessary complexity.
• Use a consistent coding style and adhere to the project's established coding standards.
• Include relevant documentation with your changes.
• Request reviews from appropriate team members and allow sufficient time for them to review the code.
• Be open to feedback and use it to improve your code.
• Respond to comments and questions from reviewers promptly.
• Make sure that all pull request requirements are met before merging changes into the main branch.

Pull Requests

Write a short paragraph of what this pull request does. Give the Peer Reviewer an overview of what they should be reviewing. This doesn't have to be more than 2-3 lines.

• Does it fix a functional bug or fix a user interface or user experience problem?
• Does it add or remove functionality?
• Does it enhance code or code readability by way of refactoring?

WIP Pull Requests

If you are creating a Pull Request that is a Work In Progress, prefix it with WIP at the beginning of your PR title.

[karltaylor]

What about associating PR's to Issues?

How should we incorporate different processes if we move to a different issue tracker e.g jira (lol) or https://height.app/ or https://clickup.com/

[karltaylor]

• If any additions add specific functionality enhancements to build process and/or developer experience, ensure that all corresponding documentation is added to the README.md - @SamScott3000

[karltaylor]

• Mini checklist for PR's on all projects (Use them as comments in the template) - @kkupcin, (Actionable Checklist). If unable to check, explanation of why.
• Make this actionable, quick, and easy to complete (not tedious)

[karltaylor]

• Link to the original issue/ticket

[karltaylor]

• Screen Size Tests (In Template)

[karltaylor]

• In the docs, mention that all comments from PR's are to be taken at face value.
• Use the "Suggestion" button from Github to give quick code fixes.

[pvalentim]

My 2 cents on this:

• Description/Summary of the PR should always come from the issue/ticket associated with it. (this is part of a bigger organisation change that will happen soon)
• Checklists are ok but I would keep the default to a minimum of essential things, the person creating the PR can always add more.
• PRs shouldn't be large or long lived
• Visual QA is very important and github ui is not great for that, what could we do ?
• Our templates should have github actions for linting and running tests so every PR get's checked automatically

[pvalentim]

Seems like pending vercel comments get surfaced on github automatically for visual QA:
https://vercel.com/docs/concepts/deployments/comments/integrations