Processing Contributions

Attracting valuable contributions

We get a lot of pull requests and other contributions, both solicited and unsolicited. The earlier we can channel the "contribution energy" that is on offer, the less work it will generally be to process the result, such as a pull request. Hence, we should clearly communicate what kind of contributions we need at any given moment.

What we can do on this front:

• Keep the roadmap up-to-date
  • Ensure that every item on the roadmap has an itemized TODO-list and a clear path to contributing
• Keep a shortlist of issues to address and other stuff to do
• Call out and thank contributors in Discord when we merge their PRs

You are not your code. If we give feedback or reject the PR it's not your fault, no insult. We just have to be narrowly focused. ~ @erik-megarad

From pull request to merge commit

Note
Maybe this is not your favorite activity (yet), although it can be rewarding to see an item being handled after you marked it as 🏷️ Promising or brought it up to be addressed with priority. And sometimes, it just feels good to sweep-close a batch of pull requests or issues that don't add any value, freeing up capacity in the team for more important things.

Pre-filtering

Who: Catalysts
What: all pull requests with no recent Catalyst/Maintainer review and no milestone

We can process a pull request if it satisfies the following conditions:

• It has a description that clearly states the reason for submitting and reflects the scope of the proposed changes
• It fits the project scope (e.g. not a PR that adds functionality which should be in a plugin)
• It has not been superseded by another PR
• It is not obviously worthless or invalid
• The proposed changes were not completely generated by an LLM
• If an LLM was used in the making of the pull request, this is clearly stated

If a pull request fails one of these conditions, it can be closed, with a comment that explains why we can't accept it and if appropriate a reference to these guidelines. No need to be rude, but also don't feel obliged to accommodate clowns, bots, or other time wasting entities.

If the pull request passes these basic requirements, it is accepted and can be entered into the PR pipeline:

1. Label it
2. Add it to the kanban's 🏷️ Promising column
3. If it has obvious areas of improvement, leave a review with a Request For Changes

Prioritizing

Who: Catalysts + Maintainers
What: accepted pull requests with no milestone

In order of importance, we can prioritize pull requests by the following properties:

1. Patches an immediate security or safety risk
2. Fixes recently introduced UX regression
3. Fixes a significant UX issue
4. Resolves a common issue or feature request
5. Gives significant progress towards a roadmapped objective
6. Is of obvious high quality
   • has a good description
   • has linked the issues that it addresses

Beyond the above, it is a practical exercise of finding PRs with high ROI, either because they are of high value or because it costs very little work to integrate them.

If you are not sure how to judge a pull request, feel free to ask a more senior Catalyst or a Maintainer to lend you a second pair of eyes.

Priority should be assigned to the card on the kanban, and urgent items should be brought to maintainers' attention. The PR should also be assigned to a milestone based on its priority.

Testing

Who: Catalysts, Community Testers
What: pull requests that need manual testing

Some changes are completely covered by our test suite, which is run automatically in the CI pipeline for every pull request. Other changes must be tested manually, to ensure they have the desired effect and do not cause regression.

Examples:

• changes that require performance/efficacy evaluation
  • changes to the prompt
  • new commands or changes to command signatures
• changes that require manual testing for regression/breakage
  • code that interacts with other programs on the system (such as browsers) needs cross-platform testing
  • changes that only affect behavior after x cycles

If manual testing is necessary and the author has not included any tests + results with their PR, we can save ourselves some time by asking the author to test their changes first and post the results.

For help with testing, we have a 🧪 • testing channel on Discord. There we can ask for help from the community to test pull requests or functions on the various supported platforms.

Reviewing & merging

Who: Maintainers
What: pull requests that are urgent or assigned to a milestone

The final "hurdle" is to pass the Maintainers' scrutiny. In the Maintainer guide you can read what their starting point is when reviewing contributions.

Steps:

1. Review the code with special regard for project context
2. Request/make changes if necessary
3. Engage in discussion with the author or with team members if something is unclear
4. Pass final judgement by merging or closing the pull request
5. After merging:
   • Post a message that the PR has been merged in 🔄️ • pull-requests and thank the author
   • Give the author a @Contributor badge on Discord

Note
Although maintainers perform final review, they are still bound by branch protection rules. So before merging:

• All required tests must pass
• All conversations on the PR and its code must be resolved
• The PR must be approved by a maintainer; maintainers can not approve their own PR

Conventions

• Prefix a comment with nit: if we're reviewing someone's code but don't want to insist they take our suggestion.
• Assign yourself to a pull request or issue when picking it up, to show what you're up to and so people can see who to contact with questions or input.