Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

WorkingOnWeave

Jump to bottom
Ciaran Moran edited this page Oct 11, 2021 · 43 revisions

Outline Development Process

We use pull requests to keep as clean a history as possible in the main (weaveworks) repository.

Members of the Weaveworks GitHub organisation use branches in the main weaveworks repo. Others fork the main repo into their own repo.

To set things up, add a remote for the main repo. I call mine 'upstream'.

$ git remote add upstream git@github.com:weaveworks/weave
$ git remote set-url --push upstream NO_PUSH

The second line above is to prevent accidental pushes -- you can always reinstate the regular URL, for special circumstances, or use the repo URL directly.

In practice this means:

  • work on feature branches for all but the simplest most uncontroversial changes
  • before you start a feature branch, make sure the branch you're branching from (probably master) is up to date with the main repo; e.g.,
$ git checkout master
$ git fetch upstream master
$ git merge --ff-only upstream/master
$ git checkout -b issuenum_feature_branch
  • If git status lists modified files under weave/vendor, then run:
$ git submodule update
  • before you submit a PR, make sure it's likely to merge cleanly, ideally as a fast-forward, to the main repo. This may involve a rebase; e.g.,
$ git fetch upstream master
$ git rebase upstream/master

Also, run all the smoke-tests in the weave/test directory.

This invariant should hold:

  • you can always fast-forward your master to upstream/master

Finally, please make sure you gofmt your code before submission. See this blog article for information on invoking gofmt from your IDE or editor of choice, or as a preventative pre-commit hook.

Code Review

  • All PRs are reviewed by at least one member of the weaveworks org.
  • PRs containing doc changes must CC @abuehrle for approval.
  • To promote a sense of shared code ownership, PRs are merged by the reviewer and not by the author.
  • To request a review from a specific individual:
    • Invite them by @mention in a PR comment; acceptance is signified by them self assigning the issue. If you are invited in this fashion and are unable to accept, please leave a PR comment to that effect.
    • Alternatively, by prior arrangement with the individual only you may assign the issue directly to them. If they are not able to handle the review promptly, they may reassign it back to you with a comment.
  • Once someone has accepted the review task the issue may be assigned back and forth freely between author and reviewer until it is merged.

Co-operating on feature branches

If co-operating on a feature branch, try to minimise the possibility of 3-way merges by only pushing when you can fast-forward. In essence, this means a similar workflow to the above: fast-forward onto the remote, do work, fetch and rebase if necessary, push.

Contributors may choose to keep feature branches in the main repository to ease collaboration, or to avail themselves of the CI system. In this case, the following convention is used for branch naming:

issues/<issue-number>-short-descriptive-text

For example:

issues/984-register-dns-resolver

Rationale: issues is used as a namespace to group feature branches in the main repo so they may be distinguished from other types of branch; use of the directory separator is transparent to git for this purpose, leaving userspace tools to represent the hierarchy as they see fit

Please remember to remove such branches from the main repo once they are merged - there is a handy button on the GitHub UI for this purpose.

Working on release branches

For each mainline release with tag vX.Y.0 there will be a release branch forked from the tag named X.Y that is used for bug fixes and updates to release documentation.

  • When you merge a PR into a release branch, you are responsible for manually merging from the release branch into master
  • GitHub does not automatically close issues that are only mentioned as fixed/closed in the PR comments when the target branch isn't master - even when you manually merge it to master afterwards. The recommended procedure is to duplicate the fixes/closed notation in your comment on the initial merge to the release branch - GitHub will do the right thing when this merge commit is subsequently merged into master.
Clone this wiki locally