On Sat, Jul 19, 2014 at 12:48 PM, Robert Samuel Newson <[email protected]> wrote: > I would like to see a formal change to R-T-C too. I include the useful notes > that Cloudant follows for this model below. Obviously not all this text > belongs in the bylaws. I suggest it, or a variant of it, belongs on our wiki > but is referenced in the bylaws, which should also cover the rules around > changing the policy. That is, the bylaws should be able to remain static > while the R-T-C page might evolve over time. > > --- > > We follows a R-T-C protocol for making code changes. Our reviews happen in > the form of Pull Requests on GitHub. Pull Requests (PRs) are a central point > of coordination around developing GitHub and as such we have a well > established policy for handling them. This document outlines the current > policies and best practices and each team member is expected to follow them. > > Characteristics of a Good Pull Request > > • A logical progression of commits > • Each commit is a single logical change > • If a commit message says "Also did XYZ while in this code" > that's a strong smell that it should be two commits > • Do not mix logic and style changes in a commit > • Ie, make whitespace changes in their own commit > • Feel free to commit often and haphazardly when writing code but use > tools to reorder and rewrite commits before issuing a Pull Request > • Learn how to do interactive rebase > • git add -p is also useful > • When responding to Pull Request comments use new commits to address > the comments so that the discussion is kept sane > • Before merging a Pull Request when all reviewers have given a > thumbs up rewrite the history of the PR and force push it to GitHub > • Pull Requests are a discussion. You don't have to take everything > every reviewer says as a command but you may be asked for a technical > argument on why you don't want to make suggested changes. > > Merging Pull Requests > > • We use a rough equivalent of Apache Voting system > • Generally a +1 is taken to mean the reviewer understands the > change, agrees with the principle behind it, and is confident that it does > what is claimed (and only what is claimed) > • Generally speaking, if one or two people +1 a change, feel free to > merge it. It's rare to merge without a single +1 > • If someone suggests a change to a PR then that blocks the merge > until either the change is made or there's an agreement on why to not make > the change > • If you suggest a change on a PR it can be helpful to illustrate the > level of care you have for the change > • Style changes are usually low priority (unless they're > egregiously bad violations) so things like "I disagree with this variable > name" shouldn't be merge-stopping issue unless there's a good reason (ie, > don't use a variable "count" to store an average) > • Algorithmic changes are generally more important > • Its also perfectly valid to suggest a change with a "Feel > free to ignore this" disclaimer. Gathering reactions to code in the > discussion is important even if we don't act on them > • Use the 'merge' button. > • Do not merge a branch onto itself. > • Never move a tag once made public, burn a version number if you > have to instead > • Don't delete the topic branch after merge > > —— > > Proper Commit Messages > > First, read > http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html > > We pride ourselves on having valuable and useful commit messages. This means > that we adhere to a standard and make use of the various sections of a commit > message for tooling and other automated capturing of our development history. > > Its important to remember that a Git commit message is one of the most > permanent things a developer will create in their day to day activities. If > the commit message is not useful then it is wasting precious opportunity for > conveying historical information about the project. > > * Commit messages should reference JIRA tickets > * Bad commit messages are grounds for veto > > — >
Awesome! It would be good to see such guidelines and more of them. -- ,,,^..^,,,
