|Version 23 (modified by 5 years ago) ( diff ),|
We use Phabricator to handle all code contributions, both from contributors and team members. This platform allows us to perform reviews in an efficient way, and it also makes automated testing of patches easy. This guide gives you details about our workflow, please read it.
At any point in the process, it is advisable to drop on the IRC channel to discuss your code or just to chat with us.
As of today, we still use Trac as a wiki and bugtracker while Phabricator is only used to manage the code.
NB: This process isn't well suited for art changes, it's best to use the forums to submit new proposed art.
Making some changes
First you should check out the game code from SVN or an up-to-date Git fork and make some changes.
Ideally, those changes are already described in one of our Trac tickets, so you know it is wanted. It would be a shame if you put some effort in a code that we actually don't want. If your change is a feature, it needs to have a ticket associated so we can discuss whether the feature is wanted and how. If it is a bugfix or a tiny change in the code, it's not mandatory to create a new ticket.
Make sure you're on the latest version of the code (and merged in your own changes) when making your patch. And try to follow the coding conventions.
If this is your first contribution, please add yourself to the relevant file under ps/trunk/binaries/data/mods/public/gui/credits/texts. You are free to use your real name, your nickname or both.
Always try to keep your patches reasonable. Different bugs shouldn't be mixed in the same patch.
When you modify or add methods to simulation components, you should add unit tests that test all added or modified changes.
Add changes in patches with their context, which allows better understanding the patch. Configure it in your SVN GUI client or add parameters in the command line:
svn diff --diff-cmd diff -x "-U 99999" > changes.patch
By sending in a patch, you agree that it is your own work (or else make it clear where it came from) and agree to licensing any code as GPL 2+ (or in some cases MIT, especially for code in
lib/ - check the existing copyright headers on the source files you edit) and data files as CC-BY-SA 3.0.
Creating a revision on Phabricator for review
You need to create a revision containing a patch and relevant information on Phabricator. Read Phabricator for the technical details.
Don't forget to put a reference to the Trac ticket when creating a revision on Phabricator (using the "Trac Tickets" field).
Update the associated Trac ticket (if relevant)
Notify the followers of the Trac ticket by posting an update on it:
- Put a link to the Differential revision in the Patch field. You can type
Phab:D11and it will render like this: Phab:D11.
- Change the Milestone from Backlog to Work In Progress, if relevant. The Alpha milestones are meant to plan our next version, so don't touch them. When the ticket is closed as fixed, we will change the Milestone from Work In Progress to the upcoming Alpha.
- You can also add a comment to the ticket at the same time.
Upon uploading the patch, an automated build, along with a number of tests, will be run on your change. Wait for them to complete and, if it fails, try to address the issue. If it is successful, wait a bit for the patch to get a reviewer assigned to it. If it doesn't in the following few days, don't hesitate to ping us over IRC.
Some patches can take longer to review due to high complexity, lower priority or other circumstances. But it's best to keep in touch with us and ask for reviews.
The reviewer will test your patch and give you comments. When it is ready, it should be committed. However it is not the end of the journey! If we discover later that your patch had a hidden flaw, we will ask you to work on a fix. Usually, the reviewer who accepted your patch will contact you.
While your patch awaits for review, and if you are interested in contributing, you are encouraged to review other people's patches! We have review guidelines at ReviewingPatches, and you can also just test the patches.
What changed with the new workflow?
The patch submission workflow recently changed after we set up Phabricator for code reviews.
The patches currently on Trac should be re-submitted by their authors following the guidelines above.
We don't need anymore the [PATCH] tag or the patch, review, rfc keywords, since no code contribution should be reviewed on Trac. Remove them when you port a patch to Phabricator, but there is no need to batch modify old tickets.
We would also like to preserve the current review/rfc queues so we know what still needs to be ported to Phabricator. They will be deleted when they are "naturally" empty.