Skip to content

Collaborative development model

Hilmar Lapp edited this page Jul 24, 2016 · 3 revisions

Introduction

This document is meant to lay out general rules and workflows for collaborative development within Phyloref. It is here because we expect it to apply across Phyloref repositories, and so that it can therefore be referenced from repository-specific CONTRIBUTING.md documents, keeping those less redundant. We also expect it to apply equally to internal as well as external contributors.

Collaborative development model

We generally follow the Feature Branch Model. Project-internal contributors may push feature branches directly against a Phyloref repo, or they work off of a forked repo as external contributors typically would, but the development model is otherwise the same.

The following lists our best practice recommendations for pull requests. If you submit a pull request that we think doesn't follow these, expect that we ask you to make corresponding changes.

  • Always create a feature branch, including if you work off of a fork. This is implied by the Feature Branch Model.

  • Keep pull requests (and thus feature branches) single-scoped. Pull requests mixing multiple scopes are much harder to review, and defeat using pull request history as a (even if imperfect) proxy for a change log.

    Scope is a squishy term. One way to think about scope is a set of changes that stands on its own by, taken together, implementing a useful and meaningful piece of functionality when merged into master. Usually, the description of the pull request will say what this is, if it's not already evident from the title. These pieces of functionality can range from entirely mundane, such as improved housekeeping brought about by a single line in a configuration file, to a highly complex feature that touches many files in the repo. If you have commits in a pull request that could be removed without compromising the "useful and meaningful piece of functionality" you've defined for it, it's a tell-tale sign that you are mixing multiple scopes.

  • Avoid having merge commits in your pull request. There is rarely if ever a benefit to having them; if you must merge into your feature branch, whether from upstream or from a branch of your own, use git rebase instead of merge.

    GitHub allows rebased branches to be pushed to already open pull requests, so rebasing is still possible for a branch even after having opened a pull request. (You will probably need to use --force when pushing.)

  • Expect that the developer who merges the pull request decides to do so by squashing the commits into one.

    Note that this means that once merged into master, your change set can only easily be revoked (backed out) as a whole. For single-scoped pull requests, this makes a lot of sense, and in fact backing out change sets later that consist of many commits can be cumbersome. However, it is also another reason for why your pull request should be single-scoped. If for some reason you want to insist on a merge commit that keeps every individual commit in your pull request, please expressly state that in your pull request.

  • Feature branches are considered temporary. (This is also implied by the Feature Branch Model.)

    If you work off of your own fork, obviously you get to decide when to delete the branch once a pull request is merged. But if you work directly off of the Phyloref repo, expect that the developer who merges your pull request will also delete the feature branch afterwards. Feature branches left around may be cleaned up at any time.

  • Use GitHub's ability to link between commits, issue tracker, and pull request to tie these together as much and whenever possible, creating a self-documenting map of past development. Close issues automatically from a commit or from a pull request upon merge to master if they fix the issue.

Epilogue

This is an evolving document, and the conventions and rules it proposes are subject to discussion and revision. Post an issue to the tracker if you have comments or suggestions.