github
/
vitess-gh
зеркало из https://github.com/github/vitess-gh.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Merge pull request #2268 from michael-berlin/improve_github_doc 

vitess.io: Improvements to the "Contributing" section.
This commit is contained in:
Michael Berlin 2016-11-21 18:49:35 -08:00 коммит произвёл GitHub
Родитель a762477463 eb769b3223
Коммит 198de6eee9
5 изменённых файлов: 140 добавлений и 62 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

60
doc/CodeReviews.md
Просмотреть файл

@ -1,6 +1,47 @@
# Code Reviews
Every GitHub pull request must go through a code review and approved before it will be merged into the master branch.
Every GitHub pull request must go through a code review and get approved before it will be merged into the master branch.
## What to look for in a Review
Both authors and reviewers need to answer these general questions:
* Does this change match an existing design / bug?
* Is there proper unit test coverage for this change? All changes should
increase coverage. We need at least integration test coverage when unit test
coverage is not possible.
* Is this change going to log too much? (Error logs should only happen when
the component is in bad shape, not because of bad transient state or bad
user queries)
* Does this change match our coding conventions / style? Linter was run and is
happy?
* Does this match our current patterns? Example include RPC patterns,
Retries / Waits / Timeouts patterns using Context, ...
Additionally, we recommend every author to look over your own reviews just before committing them and check if you are following the recommendations below.
We usually check these kinds of things while skimming through `git diff --cached` just before committing.
* Scan the diffs as if you're the reviewer.
* Look for files that shouldn't be checked in (temporary/generated files).
* Googlers only: Remove Google confidential info (e.g. internal URLs).
* Look for temporary code/comments you added while debugging.
* Example: fmt.Println("AAAAAAAAAAAAAAAAAA")
* Look for inconsistencies in indentation.
* Use 2 spaces in everything except Go.
* In Go, just use goimports.
* Commit message format:
* ```
<component>: This is a short description of the change.
If necessary, more sentences follow e.g. to explain the intent of the change, how it fits into the bigger picture or which implications it has (e.g. other parts in the system have to be adapted.)
Sometimes this message can also contain more material for reference e.g. benchmark numbers to justify why the change was implemented in this way.
```
* Comments
* `// Prefer complete sentences when possible.`
* Leave a space after the comment marker `//`.
During the review make sure you address all comments. Click Done (reviewable.io) or reply with "Done." (GitHub Review) to mark comments as addressed. There should be 0 unresolved discussions when it's ready to merge.
## Assigning a Pull Request
@ -24,23 +65,6 @@ Pull requests can be merged after they were approved and the Travis tests have p
External contributions will be merged by a team member.
Internal team members can merge their **own** pull requests.
## What reviewers should look for
Here are a few questions reviewers need to answer.
As the author of the pull request, you should also check for these before creating the pull request.
* Does this change match an existing design / bug?
* Is there proper unit test coverage for this change? All changes should
increase coverage. We need at least integration test coverage when unit test
coverage is not possible.
* Is this change going to log too much? (Error logs should only happen when
the component is in bad shape, not because of bad transient state or bad
user queries)
* Does this change match our coding conventions / style? Linter was run and is
happy?
* Does this match our current patterns? Example include RPC patterns,
Retries / Waits / Timeouts patterns using Context, ...
## Internal Bug Numbers
Most of the bugs the team is working on are tracked internally.

20
doc/GitHubWorkflow.md
Просмотреть файл

@ -8,10 +8,11 @@ Our GitHub workflow is a so called triangular workflow:
*Image Source:* https://github.com/blog/2042-git-2-5-including-multiple-worktrees-and-triangular-workflows
You develop and commit your changes in a clone of our upstream repository
(`local`). Then you push your changes to your forked repository (`origin`) and
send us a pull request. Eventually, we will merge your pull request back into
the `upstream` repository.
The Vitess code is hosted on GitHub (https://github.com/youtube/vitess).
This repository is called *upstream*.
You develop and commit your changes in a clone of our upstream repository (shown as *local* in the image above).
Then you push your changes to your forked repository (*origin*) and send us a pull request.
Eventually, we will merge your pull request back into the *upstream* repository.
## Remotes
@ -71,7 +72,10 @@ $ git checkout master
```
Try to commit small pieces along the way as you finish them, with an explanation
of the changes in the commit message. As you work in a package, you can run just
of the changes in the commit message.
Please see the [Code Review page](/contributing/code-reviews.html) for more guidance.
As you work in a package, you can run just
the unit tests for that package by running `go test` from within that package.
When you're ready to test the whole system, run the full test suite with `make
@ -123,5 +127,7 @@ $ git checkout new-feature
That is because a pull request always mirrors all commits from your topic branch which are not in the master branch.
After you merge, close the issue (if it wasn't automatically closed) and delete
the topic branch.
Once your pull request is merged:
* close the GitHub issue (if it wasn't automatically closed)
* delete your local topic branch (`git branch -d new-feature`)

80
docs/contributing/code-reviews.html
Просмотреть файл

@ -278,7 +278,66 @@
<div id="centerTextCol">
<h1 id="code-reviews">Code Reviews</h1>
<p>Every GitHub pull request must go through a code review and approved before it will be merged into the master branch.</p>
<p>Every GitHub pull request must go through a code review and get approved before it will be merged into the master branch.</p>
<h2 id="what-to-look-for-in-a-review">What to look for in a Review</h2>
<p>Both authors and reviewers need to answer these general questions:</p>
<ul>
<li> Does this change match an existing design / bug?</li>
<li> Is there proper unit test coverage for this change? All changes should
increase coverage. We need at least integration test coverage when unit test
coverage is not possible.</li>
<li> Is this change going to log too much? (Error logs should only happen when
the component is in bad shape, not because of bad transient state or bad
user queries)</li>
<li> Does this change match our coding conventions / style? Linter was run and is
happy?</li>
<li> Does this match our current patterns? Example include RPC patterns,
Retries / Waits / Timeouts patterns using Context, ...</li>
</ul>
<p>Additionally, we recommend every author to look over your own reviews just before committing them and check if you are following the recommendations below.
We usually check these kinds of things while skimming through <code class="prettyprint">git diff --cached</code> just before committing.</p>
<ul>
<li> Scan the diffs as if you&#39;re the reviewer.
<ul>
<li> Look for files that shouldn&#39;t be checked in (temporary/generated files).</li>
<li> Googlers only: Remove Google confidential info (e.g. internal URLs).</li>
<li> Look for temporary code/comments you added while debugging.
<ul>
<li> Example: fmt.Println(&quot;AAAAAAAAAAAAAAAAAA&quot;)</li>
</ul></li>
<li> Look for inconsistencies in indentation.
<ul>
<li> Use 2 spaces in everything except Go.</li>
<li> In Go, just use goimports.</li>
</ul></li>
</ul></li>
<li><p>Commit message format:</p>
<ul>
<li><div class="highlight"><pre><code class="language-text" data-lang="text">&lt;component&gt;: This is a short description of the change.
If necessary, more sentences follow e.g. to explain the intent of the change, how it fits into the bigger picture or which implications it has (e.g. other parts in the system have to be adapted.)
Sometimes this message can also contain more material for reference e.g. benchmark numbers to justify why the change was implemented in this way.
</code></pre></div></li>
</ul></li>
<li><p>Comments</p>
<ul>
<li> <code class="prettyprint">// Prefer complete sentences when possible.</code></li>
<li> Leave a space after the comment marker <code class="prettyprint">//</code>.</li>
</ul></li>
</ul>
<p>During the review make sure you address all comments. Click Done (reviewable.io) or reply with &quot;Done.&quot; (GitHub Review) to mark comments as addressed. There should be 0 unresolved discussions when it&#39;s ready to merge.</p>
<h2 id="assigning-a-pull-request">Assigning a Pull Request</h2>
@ -304,25 +363,6 @@ They&#39;ll receive an email.</p>
External contributions will be merged by a team member.
Internal team members can merge their <strong>own</strong> pull requests.</p>
<h2 id="what-reviewers-should-look-for">What reviewers should look for</h2>
<p>Here are a few questions reviewers need to answer.
As the author of the pull request, you should also check for these before creating the pull request.</p>
<ul>
<li> Does this change match an existing design / bug?</li>
<li> Is there proper unit test coverage for this change? All changes should
increase coverage. We need at least integration test coverage when unit test
coverage is not possible.</li>
<li> Is this change going to log too much? (Error logs should only happen when
the component is in bad shape, not because of bad transient state or bad
user queries)</li>
<li> Does this change match our coding conventions / style? Linter was run and is
happy?</li>
<li> Does this match our current patterns? Example include RPC patterns,
Retries / Waits / Timeouts patterns using Context, ...</li>
</ul>
<h2 id="internal-bug-numbers">Internal Bug Numbers</h2>
<p>Most of the bugs the team is working on are tracked internally.

22
docs/contributing/github-workflow.html
Просмотреть файл

@ -286,10 +286,11 @@
<p><em>Image Source:</em> <a href="https://github.com/blog/2042-git-2-5-including-multiple-worktrees-and-triangular-workflows">https://github.com/blog/2042-git-2-5-including-multiple-worktrees-and-triangular-workflows</a></p>
<p>You develop and commit your changes in a clone of our upstream repository
(<code class="prettyprint">local</code>). Then you push your changes to your forked repository (<code class="prettyprint">origin</code>) and
send us a pull request. Eventually, we will merge your pull request back into
the <code class="prettyprint">upstream</code> repository.</p>
<p>The Vitess code is hosted on GitHub (<a href="https://github.com/youtube/vitess">https://github.com/youtube/vitess</a>).
This repository is called <em>upstream</em>.
You develop and commit your changes in a clone of our upstream repository (shown as <em>local</em> in the image above).
Then you push your changes to your forked repository (<em>origin</em>) and send us a pull request.
Eventually, we will merge your pull request back into the <em>upstream</em> repository.</p>
<h2 id="remotes">Remotes</h2>
@ -331,7 +332,10 @@ this, run this command once:</p>
(new-feature) $ # You are now in the new-feature branch.
</code></pre></div>
<p>Try to commit small pieces along the way as you finish them, with an explanation
of the changes in the commit message. As you work in a package, you can run just
of the changes in the commit message.
Please see the <a href="/contributing/code-reviews.html">Code Review page</a> for more guidance.</p>
<p>As you work in a package, you can run just
the unit tests for that package by running <code class="prettyprint">go test</code> from within that package.</p>
<p>When you&#39;re ready to test the whole system, run the full test suite with <code class="prettyprint">make
@ -370,8 +374,12 @@ another commit on your branch and then push it again:</p>
</code></pre></div>
<p>That is because a pull request always mirrors all commits from your topic branch which are not in the master branch.</p>
<p>After you merge, close the issue (if it wasn&#39;t automatically closed) and delete
the topic branch.</p>
<p>Once your pull request is merged:</p>
<ul>
<li> close the GitHub issue (if it wasn&#39;t automatically closed)</li>
<li> delete your local topic branch (<code class="prettyprint">git branch -d new-feature</code>)</li>
</ul>
</div>
</div>

20
docs/sitemap.xml
Просмотреть файл

@ -21,20 +21,11 @@
<url>
<loc>http://vitess.io/user-guide/horizontal-sharding.html</loc>
</url>
<url>
<loc>http://vitess.io/about/</loc>
</url>
<url>
<loc>http://vitess.io/search/</loc>
</url>
<url>
<loc>http://vitess.io/overview/</loc>
</url>
<url>
<loc>http://vitess.io/getting-started/</loc>
</url>
<url>
<loc>http://vitess.io/contributing/</loc>
<loc>http://vitess.io/about/</loc>
</url>
<url>
<loc>http://vitess.io/terms/</loc>
@ -42,6 +33,15 @@
<url>
<loc>http://vitess.io/</loc>
</url>
<url>
<loc>http://vitess.io/search/</loc>
</url>
<url>
<loc>http://vitess.io/getting-started/</loc>
</url>
<url>
<loc>http://vitess.io/contributing/</loc>
</url>
<url>
<loc>http://vitess.io/user-guide/introduction.html</loc>
</url>