-
Notifications
You must be signed in to change notification settings - Fork 184
Submitting, Reviewing & Merging Pull Requests
This document provides an overview of what to expect after submitting a PR and what folks who are in charge of reviewing PRs should be mindful of before merging changes in to the main branch (at which point the "public" itch.io build is updated).
The contributor opens a "Pull Request", which contains the relevant changes to the game's code/assets. This kicks off a few automated processes:
- A new web build is created and published (takes a few seconds - be patient!)
- A comment is added to the PR with a link to the new test build
Even if the game was working fine on your local machine, we encourage you to open the test build link and make sure it's still functioning as expected. You can always update your PR and test again with a new build link!
Be mindful that the main branch may have been updated since you finished your changes, so there may be merge conflicts. It's usually a good idea to pull the most recent changes down immediately PRIOR to submitting a PR to reduce the chances of this happening.
Be sure to do small, incremental PRs rather than one big one! Smaller changes are much less likely to have merge conflicts and much MORE likely to be merged into main with minimal delay. Also - avoid renaming/refactoring files in the same PR that functionality changes are being made in, as this will likely also cause problems for many other folks (refactoring in general is likely to be rejected unless it has a major benefit to the project at large).
If you have been granted the "Maintain" role, you can review and merge PRs. Let's talk about reviewing first.
You will not be able to review your own PRs, but you can act as a reviewer for other open PRs. This means that you can "approve" a PR and allow yourself or the PR author to merge it into the main branch (again, assuming there are no conflicts).
Some things to be mindful of while reviewing a PR:
- Try out the test build! Keep an eye out specifically for anything that could have been impacted by the changes described in the PR
- If there are merge conflicts, you may want to simply ask the author to update locally and resubmit. You can resolve them yourself if they're simple and/or you're comfortable dealing with conflicts, but don't feel like that responsibility falls on you!
- Don't approve changes that you don't feel confident that you can assess adequately - if it's something a little too complex, don't hesitate to request someone more experienced to look at it instead.
- Don't approve changes that only refactor (reformat) existing code. Some folks will just want to "clean" things up, but this is often more hassle than its worth for a large, shared project like this. Use your best judgment and when in doubt, ask another reviewer for their opinion!
Once a PR has been reviewed, it should be good to be merged into main! A short time after changes are merged in, the build on itch.io will be updated. If you followed all the above guidelines, it should be fine...but it doesn't hurt to check on it, just in case. 😉
Please ask in the Discord if you are interested in helping to triage PR reviewing and merging! Our preference is that you've made at least made one or two contributions of your own and have some level of comfort with GitHub, but we're also willing to help new folks learn if you're really interested. Thank you!