Skip to content

Commit

Permalink
copyediting and minor style changes, typos
Browse files Browse the repository at this point in the history
  • Loading branch information
sneak committed Sep 13, 2016
1 parent a981a5a commit 7dc34e3
Showing 1 changed file with 40 additions and 38 deletions.
78 changes: 40 additions & 38 deletions doc/git-guildelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,26 @@ post](http://www.draconianoverlord.com/2013/09/07/no-cherry-picking.html).

## Branches

- `origin/master` : Contains releases of Steem. `origin/master` and
`origin/HEAD` should always be at the most recent release. All witnesses
should be running this branch. Release commits will have a label
- `master`: Points to the current release of Steem. Witnesses should be
running this branch. Each release commit will be tagged
`vMajor.Hardfork.Release`. When we get ready to release we will merge
feature branches into develop and then do a single merge into master. Pull
Requests must pass automated testing to be merged into master.
- `origin/develop` : The development branch. We will strive to keep develop
as a working branch as much as possible. All branches must pass automated
tests before being merged. While this is not a guarantee that develop is
bug free, it will guarantee to the best of our knowledge that it is safe
to build from develop. That being said, running a node from develop has
risks. We recommend that any block producing node build from master. If
you want to test new features, develop is the correct branch.
- `origin/staging` : The purpose of staging was to be a more stable version
of develop. However, with the requirement to pass automated testing to
merge into develop, the purpose of staging is not longer relevent. This
branch is now defunct.
feature branches into `develop` and then do a single merge into `master`
via a Pull Request. All PRs to `master` must pass automated testing to be
merged.
- `develop`: The active development branch. We will strive to keep `develop`
in a working state. All PRs must pass automated tests before being merged.
While this is not a guarantee that `develop` is bug-free, it will
guarantee that the branch is buildable in our standard build configuration
and passes the current suite of tests. That being said, running a node
from `develop` has risks. We recommend that any block producing node
build from `master`. If you want to test new features, develop is the
correct branch.

### Patch Branches

All issues should be developed against their own branch. These branches
should base off `develop` and then merged back into develop when they are
tested and ready to be deployed. If an issue needs another issue as a
All changes should be developed in their own branch. These branches
should branch from `develop` and then merged back into `develop` when they are
tested and ready. If an issue needs another issue as a
dependency, branch from `develop`, merge the dependent issue branch into the
new branch, and begin development. The naming scheme we use is the issue
number, then a dash, followed by a shorthand description of the issue. For
Expand All @@ -40,24 +37,29 @@ example, issue 22 is to allow the removal of an upvote. Branch

Some changes are so minor as to not require an issue, e.g. changes to
logging. Because the requirement of automated testing, create an issue for
them. We air on the side of overdocumentation rather than
underdocumentation. Branch from `develop`, make your fix, and create a pull
them. We err on the side of overdocumentation rather than
underdocumentation. Branch from `develop`, make your fix, and create a pull
request into `develop`.

Suggested formatting for such branches missing an issue is
`YYYYMMDD-shortname`, e.g. `20160913-documentation-update`. (The date in
the branch is so that we can prune old/defunct ones periodically to keep the
repo tidy.)

## Pull Requests

All merges into `develop` and `master` are done through pull requests. This
is done for several reasons:
All changes to `develop` and `master` are done through GitHub Pull Requests
(PRs). This is done for several reasons:

1. It enforces two factor authentication. Github will only allow merging of a
1. It enforces two factor authentication. GitHub will only allow merging of a
pull request through their interface, which requires the dev to be logged
in.
1. If enforces testing. All pull requests undergo automated testing they are
allowed to be merged.
1. If enforces testing. All pull requests undergo automated testing before
they are allowed to be merged.
1. It enforces best practices. Becuase of the cost of a pull request,
developers are encouraged to do more testing themselves and be certain of
the correctness of their solutions.
1. If enforces code review. All pull request must be reviewed by a developer
1. If enforces code review. All pull requests must be reviewed by a developer
other than the creator of the request. Pull requests made by external
developers should be reviewed by two internal developers. When a developer
reviews and approves a pull request they should +1 the request or leave a
Expand Down Expand Up @@ -91,19 +93,19 @@ pull request.
### Code review policy

- Two developers *must* review *every* release before merging it into
`origin/master`, enforced through pull requests.
`master`, enforced through pull requests.
- Two developers *must* review *every* consensus-breaking change before it
moves into `origin/develop`, enforced through pull requests.
moves into `develop`, enforced through pull requests.
- Two developers *should* review *every* patch before it moves into
`orogin/develp`, enforced through pull requests.
`develop`, enforced through pull requests.
- One of the reviewers may be the author of the change.
- This policy is designed to encourage you to take off your "writer hat" and
put on your "critic/reviewer hat." If this was a patch from an unfamiliar
community contributor, would you accept it? Can you understand what the
patch does and check its correctness based only on its commit message and
diff? Does it break any existing tests, or need new tests to be written?
Is it stylistically sloppy -- trailing whitespace, multiple unrelated
changes in a single patch, mixing bugfixes and features, or overly verbose
debug logging?
put on your "critic/reviewer hat." If this were a patch from an
unfamiliar community contributor, would you accept it? Can you understand
what the patch does and check its correctness based only on its commit
message and diff? Does it break any existing tests, or need new tests to
be written? Is it stylistically sloppy -- trailing whitespace, multiple
unrelated changes in a single patch, mixing bugfixes and features, or
overly verbose debug logging?
- Having multiple people look at a patch reduces the probability it will
contain bugs.
contain uncaught bugs.

0 comments on commit 7dc34e3

Please sign in to comment.