Tips to Getting Your Code Merged
11 Feb 2014[Note: the below is adapted from a post on Google+ by one of Omni's Lead Developers, Andrew Dodd aka Entropy512.]
While we have previously have included on the wiki a few notes about contributing to Omni, here are just a few observations regarding Omni code contribution, and some tips on getting your stuff merged:
- Indicate in the review comments what a patch is supposed to fix and, ideally, how to reproduce the problem or test the patch. We personally hate blind-merging stuff that hasn't been tested to actually do something.
- If it's device-specific, indicate what issue on that device it's supposed to fix.
- If it's a cherry-pick from elsewhere, DO NOT edit the commit message. Just put in a review comment after uploading.
- If you put a patch up and don't add reviewers with merge rights, it's probably not going anywhere.
- Unfortunately it can sometimes be difficult to tell who to add to review. A safe bet is to add a few people from the Contributors list.
- If you put up a lot of patches at once, you might overload the reviewers. If you put up a lot of patches that don't actually depend on each other into a gerrit dependency chain, you're going to cause massive delays if some of your patches are good but one of the earlier patches has issues.
- As an example, right now there's a good patch which is marked as having a dependency on a completely broken one that breaks the build. The patch doesn't actually depend on the other one and should have been put into a separate topic branch when it was uploaded so it could be merged on its own.
- DO NOT put someone else's existing patch into a dependency chain after one of your own patches without talking to them or catching one of those with merge rights on IRC. Seriously, just do NOT do it. Someone mass-submitted 30+ patches to Gerrit all at once two weeks ago, and took other people's patches and inserted them at the end of the dependency chain. Many of that submitter's patches were questionable/high-risk so it delayed the less-risky patches.
Regarding PM's on IRC: Please do not PM a developer without first pinging them in a public channel to see if they're there. Also, when asking if you can PM them, ask yourself, "does this really need to be private"? Most of the things people contact developers about don't need to be a PM, and if they ping them when they're asleep, they won't be helped while someone else could have helped them in IRC. Plus one of the biggest goals of Omni is transparency with developers and users, so it's better if conversations occur in public unless they REALLY have a good reason to be private.
Image courtesy of RocketModule