Skip to content

Commit

Permalink
doc: precision, concision, elision
Browse files Browse the repository at this point in the history
Single point of truth (SPOT): Do not sprinkle "contributing" guidelines
across various resources; CONTRIBUTING.md is the authority.

DRY: Do not repeat guidelines in CONTRIBUTING.md which are covered by
ISSUE_TEMPLATE.md
  • Loading branch information
justinmk committed Jun 3, 2016
1 parent f421757 commit b3b25c2
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 157 deletions.
229 changes: 80 additions & 149 deletions CONTRIBUTING.md
Expand Up @@ -3,160 +3,96 @@
## Getting started

- Help us review [open pull requests](https://github.com/neovim/neovim/pulls)!
- Look for [entry-level issues][entry-level] to work on.
- [Documentation](https://github.com/neovim/neovim/labels/documentation)
improvements are also much appreciated.
- Look at [Waffle][waffle] to see who is working on what issues.
- If needed, refer to [the wiki][wiki-contributing] for guidance.
See [Reviewing](#reviewing) for guidelines.
- Try an [entry-level issue][entry-level] if you are wondering where to start.
- Or [merge a Vim patch].

## Reporting problems

Before reporting an issue, see the following wiki articles:
- Check the [**FAQ**][wiki-faq].
- Search [existing issues][github-issues] (including closed!)
- Update Neovim to the latest version to see if your problem persists.
- If you're using a plugin manager, comment out your plugins, then add them back
in one by one, to narrow down the cause of the issue.
- Crash reports which include a stacktrace are 10x more valuable.
- [Bisecting][git-bisect] to the cause of a regression often leads to an
immediate fix.

## Pull requests ("PRs")

- To avoid duplicate work, you may want to create a `[WIP]` pull request so that
others know what you are working on.
- Avoid cosmetic changes to unrelated files in the same commit: extra noise
makes reviews more difficult.
- Use a [feature branch][git-feature-branch] instead of the master branch.
- [Rebase your feature branch][git-rebasing] onto (upstream) master before
opening the PR.
- After addressing the review comments, it's fine to rebase and force-push to
your review.
- Try to [tidy your history][git-history-rewriting]: combine related commits
with interactive rebasing, separate monolithic commits, etc.

- [Troubleshooting][wiki-troubleshooting]
- [Frequently asked questions][wiki-faq]
### Stages: WIP, RFC

If your issue isn't mentioned there:
Pull requests are processed in two stages: _WIP_ (Work In Progress) and _RFC_
(Request For Comment).

- Verify that it hasn't already been reported.
- If not already running the latest version of Neovim, update to it to see if
your problem persists.
- If you're experiencing compile or runtime warnings/failures, try searching for
the error message(s) you received (if any) on [Neovim's issue tracker][github-issues].
- For runtime issues, try reproducing it using `nvim` with the smallest
possible `vimrc` (or none at all via `nvim -u NONE`), to rule out bugs in
plugins you're using. If you're using a plugin manager, comment out your
plugins, then add them back in one by one.
- Untagged PRs are assumed to be RFC, meaning the work is ready for review and
you would like feedback.
- Preprend `[WIP]` to the PR title if you are _not_ ready for feedback and the
work is still in flux. This saves time and confusion.

Include as much detail as possible; we generally need to know:
### Commit messages

- What operating system you're using.
- Which version of Neovim you're using. To get this, run `nvim --version` from
a shell, or run `:version` from inside `nvim`.
- Whether the bug is present in Vim (not Neovim), and if so which version of
Vim. It's fine to report Vim bugs on the Neovim bug tracker, but it saves
everyone time if we know from the start that the bug is not a regression
caused by Neovim.
- This isn't required, but what commit introduced the issue for you. You can
use [`git bisect`][git-bisect] for this.
Follow [commit message hygiene][hygiene] to *make reviews easier* and to make
the VCS/git logs more valuable.

## Submitting contributions
- Try to keep the first line under 72 characters.
- **Prefix the commit subject with a _scope_:** `doc:`, `test:`, `foo.c:`,
`runtime:`, ...
- For commits that contain only style/lint changes, a single-word subject
line is preferred: `style` or `lint`.
- A blank line must separate the subject from the description.
- Use the _imperative voice_: "Fix bug" rather than "Fixed bug" or "Fixes bug."

- Make it clear in the issue tracker what you are working on.
- Be descriptive in your pull request description: what is it for, why is it
needed, etc.
- Do ***not*** make cosmetic changes to unrelated files in the same pull
request. This creates noise, making reviews harder to do. If your text
editor strips all trailing whitespace in a file when you edit it, disable
it.
### Automated builds (CI)

### Tagging in the issue tracker
Each pull request must pass the automated builds ([travis CI] and [quickbuild]).

When submitting pull requests (commonly referred to as "PRs"), include one of
the following tags prepended to the title:
- CI builds are compiled with [`-Werror`][gcc-warnings], so if your PR
introduces any compiler warnings, the build will fail.
- If any tests fail, the build will fail.
See [Building Neovim#running-tests][wiki-run-tests] to run tests locally.
Passing locally doesn't guarantee passing the CI build, because of the
different compilers and platforms tested against.
- CI runs [ASan] and other analyzers. To run valgrind locally:
`VALGRIND=1 make test`
- The `lint` build ([#3174][3174]) checks modified lines _and their immediate
neighbors_. This is to encourage incrementally updating the legacy style to
meet our style guidelines.
- A single word (`lint` or `style`) is sufficient as the subject line of
a commit that contains only style changes.
- [How to investigate QuickBuild failures](https://github.com/neovim/neovim/pull/4718#issuecomment-217631350)

- `[WIP]` - Work In Progress: the PR will change, so while there is no
immediate need for review, the submitter still might appreciate it.
- `[RFC]` - Request For Comment: the PR needs reviewing and/or comments.
- `[RDY]` - Ready: the PR has been reviewed by at least one other person and
has no outstanding issues.
### Coverity

Assuming the above criteria has been met, feel free to change your PR's tag
yourself, as opposed to waiting for a contributor to do it for you.
[Coverity](https://scan.coverity.com/projects/neovim-neovim) runs against the
master build. If you want to view the defects, just request access at the
_Contributor_ level. An Admin will grant you permission.

### Branching & history
Use this commit-message format for coverity fixes:

- Do ***not*** work on your PR on the master branch, [use a feature branch
instead][git-feature-branch].
- [Rebase your feature branch onto][git-rebasing] (upstream) master before
opening the PR.
- Keep up to date with changes in (upstream) master so your PR is easy to
merge.
- [Try to actively tidy your history][git-history-rewriting]: combine related
commits with interactive rebasing, separate monolithic commits, etc. If your
PR is still `[WIP]`, feel free to force-push to your feature branch to tidy
your history.

### For code pull requests

#### Testing

We are unlikely to merge your PR if the Travis build fails:

- Travis builds are compiled with the [`-Werror`][gcc-warnings] flag, so if
your PR introduces any compiler warnings then the Travis build will fail.
- If any tests fail, the Travis build will fail.
See [Building Neovim#running-tests][wiki-building-running-tests] for
information on running tests locally.
Tests passing locally doesn't guarantee they'll pass in the Travis
build, as different compilers and platforms will be used.
- Travis runs [Valgrind][valgrind] for the GCC/Linux build, but you may also
do so locally by running the following from a shell: `VALGRIND=1 make test`

#### Coding style

We have a [style guide][style-guide] that all new code should follow.
However, large portions of the existing Vim codebase violate it to some
degree, and fixing them would increase merge conflicts and add noise to `git
blame`.

Weigh those costs when making cosmetic changes. In general, avoid pull
requests dominated by style changes, but feel free to fix up lines that you
happen to be modifying anyway. Fix anything that looks outright
[barbarous](http://www.orwell.ru/library/essays/politics/english/e_polit), but
otherwise prefer to leave things as they are.

For new code, run `make lint` (which runs [clint.py][clint]) to detect style
errors. It's not perfect, so some warnings may be false positives/negatives.
To have `clint.py` ignore certain cases, put `// NOLINT` at the end of the
line.

We also provide a configuration file for [`clang-format`][clang-format], which
can be used to format code according to the style guidelines. Be aware that
this formatting method might need user supervision. To have `clang-format`
ignore certain line ranges, use the following special comments:

```c
int formatted_code;
// clang-format off
void unformatted_code ;
// clang-format on
void formatted_code_again;
```

### Commit guidelines

The purpose of these guidelines is to *make reviews easier* and make the
[VCS][vcs] logs more valuable.
coverity/<id>: <description of what fixed the defect>

- Try to keep the first line under 72 characters.
- If necessary, include further description after a blank line.
- Don't make the description too verbose by including obvious things, but
don't spare clarifications for anything that may be not so obvious.
Some commit messages are pages long, and that's fine if there's no
better place for those comments to live.
- **Recommended:** Prefix logically-related commits with a consistent
identifier in each commit message. For already used identifiers, see the
commit history for the respective file(s) you're editing.
[For example](https://github.com/neovim/neovim/commits?author=elmart),
the following commits are related by task (*Introduce nvim namespace*) and
sub-task (*Contrib YCM*).
<br/> `Introduce nvim namespace: Contrib YCM: Fix style issues`
<br/> `Introduce nvim namespace: Contrib YCM: Fix build dir calculation`
- Sub-tasks can be *activity-oriented* (doing different things on the same area)
or *scope-oriented* (doing the same thing in different areas).
- Granularity helps, but it's conceptual size that matters, not extent size.
- Use the [imperative voice][imperative]: "Fix bug" rather than "Fixed bug" or "Fixes bug."

### Reviewing pull requests

Using a checklist during reviews is highly recommended, so we [provide one at
the wiki][wiki-review-checklist]. If you think it could be improved, feel free
to edit it.
where `<id>` is the Coverity ID (CID). For example see [#804](https://github.com/neovim/neovim/pull/804).

## Reviewing

To help review pull requests, start with [this checklist][review-checklist].

Reviewing can be done on GitHub, but you may find it easier to do locally.
Using [`hub`][hub], you can do the following to create a new branch with the
contents of a pull request, such as [#1820][github-pr-1820]:
Using [`hub`][hub], you can create a new branch with the contents of a pull
request, e.g. [#1820][1820]:

hub checkout https://github.com/neovim/neovim/pull/1820

Expand All @@ -165,11 +101,7 @@ commits in the feature branch which aren't in the `master` branch; `-p`
shows each commit's diff. To show the whole surrounding function of a change
as context, use the `-W` argument as well.

You may find it easier to instead use an interactive program for code reviews,
such as [`tig`][tig].

[clang-format]: http://clang.llvm.org/docs/ClangFormat.html
[clint]: clint.py
[entry-level]: https://github.com/neovim/neovim/issues?labels=entry-level&state=open
[gcc-warnings]: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
[git-bisect]: http://git-scm.com/book/tr/v2/Git-Tools-Debugging-with-Git
Expand All @@ -178,16 +110,15 @@ such as [`tig`][tig].
[git-history-rewriting]: http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
[git-rebasing]: http://git-scm.com/book/en/v2/Git-Branching-Rebasing
[github-issues]: https://github.com/neovim/neovim/issues
[github-pr-1820]: https://github.com/neovim/neovim/pull/1820
[1820]: https://github.com/neovim/neovim/pull/1820
[hub]: https://hub.github.com/
[imperative]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
[hygiene]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
[style-guide]: http://neovim.io/develop/style-guide.xml
[tig]: https://github.com/jonas/tig
[valgrind]: http://valgrind.org/
[vcs]: https://en.wikipedia.org/wiki/Revision_control
[waffle]: https://waffle.io/neovim/neovim
[wiki-building-running-tests]: https://github.com/neovim/neovim/wiki/Building-Neovim#running-tests
[wiki-contributing]: https://github.com/neovim/neovim/wiki/Contributing
[ASan]: http://clang.llvm.org/docs/AddressSanitizer.html
[wiki-run-tests]: https://github.com/neovim/neovim/wiki/Building-Neovim#running-tests
[wiki-faq]: https://github.com/neovim/neovim/wiki/FAQ
[wiki-review-checklist]: https://github.com/neovim/neovim/wiki/Code-review-checklist
[wiki-troubleshooting]: https://github.com/neovim/neovim/wiki/Troubleshooting
[review-checklist]: https://github.com/neovim/neovim/wiki/Code-review-checklist
[3174]: https://github.com/neovim/neovim/issues/3174
[travis CI]: https://travis-ci.org/neovim/neovim
[quickbuild]: http://neovim-qb.szakmeister.net/dashboard
[merge a Vim patch]: https://github.com/neovim/neovim/wiki/Merging-patches-from-upstream-Vim
4 changes: 2 additions & 2 deletions ISSUE_TEMPLATE.md
@@ -1,5 +1,5 @@
- Neovim version:
- [ ] Vim behaves differently? Vim version:
- `nvim --version`:
- Vim (version: ) behaves differently?
- Operating system/version:
- Terminal name/version:
- `$TERM`:
Expand Down
8 changes: 2 additions & 6 deletions README.md
Expand Up @@ -18,7 +18,7 @@

Neovim is a project that seeks to aggressively refactor Vim in order to:

- Simplify maintenance and encourage [contributions](https://github.com/neovim/neovim/wiki/Contributing)
- Simplify maintenance and encourage [contributions](CONTRIBUTING.md)
- Split the work between multiple developers
- Enable the implementation of new/modern user interfaces without any
modifications to the core source
Expand All @@ -42,7 +42,7 @@ See the [progress page](https://github.com/neovim/neovim/wiki/Progress) for a co

### What's being worked on now

- Port all IO to [libuv](https://github.com/libuv/libuv/blob/master/README.md)
- Port all IO to [libuv](https://github.com/libuv/libuv/)
- Convert legacy tests to Lua tests
- VimL => Lua translator

Expand All @@ -51,10 +51,6 @@ See the [progress page](https://github.com/neovim/neovim/wiki/Progress) for a co
There is a formula for OSX/homebrew, a PKGBUILD for Arch Linux, RPM, deb, and
more. See [the wiki](https://github.com/neovim/neovim/wiki/Installing-Neovim)!

### Contributing

...would be awesome! See [the wiki](https://github.com/neovim/neovim/wiki/Contributing) for more details.

### License

Neovim is licensed under the terms of the Apache 2.0 license, except for
Expand Down

0 comments on commit b3b25c2

Please sign in to comment.