mirror of
https://github.com/neovim/neovim.git
synced 2024-12-19 18:55:14 -07:00
CONTRIBUTING.md: Misc improvements
- In general, many sections have been expanded a lot. - Defer to the wiki when it makes sense. - Rewrite a bunch of things to be clearer. There is a lot more verbosity, but clarifying as many items as we can in this document is preferable to it being done later in the issue tracker. (alphabetically sorted) Helped-by: David Granström <david@davidgranstrom.com> Helped-by: Florian Walch <florian@fwalch.com> Helped-by: John Szakmeister <john@szakmeister.net> Helped-by: Justin M. Keyes <justinkz@gmail.com> Helped-by: Martin Kopischke <martin@kopischke.net> Helped-by: oni-link <knil.ino@gmail.com> [ci skip]
This commit is contained in:
parent
d06b4933ed
commit
97df129956
218
CONTRIBUTING.md
218
CONTRIBUTING.md
@ -3,104 +3,190 @@
|
|||||||
## Getting started
|
## Getting started
|
||||||
|
|
||||||
- Help us review [open pull requests](https://github.com/neovim/neovim/pulls)!
|
- Help us review [open pull requests](https://github.com/neovim/neovim/pulls)!
|
||||||
- Look for [**entry-level**][entry] issues to work on.
|
- Look for [entry-level issues][entry-level] to work on.
|
||||||
- [**documentation**](https://github.com/neovim/neovim/labels/documentation)
|
- [Documentation](https://github.com/neovim/neovim/labels/documentation)
|
||||||
improvements are also very helpful.
|
improvements are also much appreciated.
|
||||||
- Look at [Waffle][waffle] to see who is working on what issues.
|
- Look at [Waffle][waffle] to see who is working on what issues.
|
||||||
- Refer to the [the wiki][wiki] for detailed guidance.
|
- If needed, refer to [the wiki][wiki-contributing] for guidance.
|
||||||
|
|
||||||
## Issues
|
## Reporting problems
|
||||||
|
|
||||||
- Search existing issues before raising a new one.
|
Before reporting an issue:
|
||||||
- Include as much detail as possible. In particular, we need to know which
|
|
||||||
OS you're using.
|
|
||||||
|
|
||||||
## Pull requests
|
- 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 installation issues, see [Installing Neovim#troubleshooting][wiki-install-troubleshooting].
|
||||||
|
- For build issues, see [Building Neovim#troubleshooting][wiki-building-troubleshooting].
|
||||||
|
- For runtime issues: try to reproduce them by running `nvim` with the
|
||||||
|
smallest possible `nvimrc` (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.
|
||||||
|
|
||||||
|
Include as much detail as possible; we generally need to know:
|
||||||
|
|
||||||
|
- 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.
|
||||||
|
|
||||||
|
## Submitting contributions
|
||||||
|
|
||||||
|
- If you're a first-time contributor, please sign the [Neovim Contributor
|
||||||
|
License Agreement (CLA)][cla] before submitting your pull request.
|
||||||
- Make it clear in the issue tracker what you are working on.
|
- Make it clear in the issue tracker what you are working on.
|
||||||
- Be descriptive in your PR message: what is it for, why is it needed, etc.
|
- Be descriptive in your pull request description: what is it for, why is it
|
||||||
- Don't make cosmetic changes to unrelated files in the same PR.
|
needed, etc.
|
||||||
- If you're a first-time contributor, please sign the
|
- Do ***not*** make cosmetic changes to unrelated files in the same pull
|
||||||
[Neovim Contributor License Agreement (CLA)][cla] before submitting your PR.
|
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.
|
||||||
|
|
||||||
#### Tagging in the issue tracker
|
### Tagging in the issue tracker
|
||||||
|
|
||||||
When submitting pull requests, include one of the following tokens in the title:
|
When submitting pull requests (commonly referred to as "PRs"), include one of
|
||||||
|
the following tags prepended to the title:
|
||||||
|
|
||||||
* `[WIP]` - Work In Progress. The pull request will change, and there is no need
|
- `[WIP]` - Work In Progress: the PR will change, so while there is no
|
||||||
to review it yet.
|
immediate need for review, the submitter still might appreciate it.
|
||||||
* `[RFC]` - Request For Comment. The request needs reviewing and/or comments.
|
- `[RFC]` - Request For Comment: the PR needs reviewing and/or comments.
|
||||||
* `[RDY]` - The request is ready to be merged. The request must have been
|
- `[RDY]` - Ready: the PR has been reviewed by at least one other person and
|
||||||
reviewed by at least one person and have no outstanding issues.
|
has no outstanding issues.
|
||||||
* Default label is assumed to be `[WIP]` if there's no indication otherwise.
|
|
||||||
|
|
||||||
#### Branching & history
|
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.
|
||||||
|
|
||||||
- Use a feature branch, not master.
|
### Branching & history
|
||||||
- Rebase your feature branch onto (upstream) master before raising the PR.
|
|
||||||
- Keep up to date with changes in (upstream) master so your PR is easy to merge.
|
|
||||||
- Try to actively tidy your history: combine related commits with interactive
|
|
||||||
rebasing etc. If your PR is still `[WIP]` don't be afraid to force-push to
|
|
||||||
your feature branch to tidy your history.
|
|
||||||
|
|
||||||
### For code PRs
|
- 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
|
#### Testing
|
||||||
|
|
||||||
- We are unlikely to merge your PR if the Travis build fails.
|
We are unlikely to merge your PR if the Travis build fails:
|
||||||
- The Travis build does not currently run the tests under valgrind, but you
|
|
||||||
are encouraged to do so locally.
|
- 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#testing][wiki-building-testing] 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
|
#### Coding style
|
||||||
|
|
||||||
We have a [style guide][style] that all new code should follow. However, vast
|
We have a [style guide][style-guide] that all new code should follow.
|
||||||
swathes of the existing vim codebase violate it to some degree, and fixing
|
However, large portions of the existing Vim codebase violate it to some
|
||||||
them would increase merge conflicts and add noise to `git blame`. Please weigh
|
degree, and fixing them would increase merge conflicts and add noise to `git
|
||||||
those costs when making cosmetic changes. As a rule of thumb, avoid pull
|
blame`.
|
||||||
requests dominated by style changes. Feel free to fix up lines that you happen
|
|
||||||
to be modifying anyway, as long as they look consistent with their
|
|
||||||
surroundings. Fix anything that looks outright
|
|
||||||
[barbarous](http://www.orwell.ru/library/essays/politics/english/e_polit) --
|
|
||||||
especially if you can't find any editor settings that make it look ok -- but
|
|
||||||
otherwise err on the side of leaving things as they are.
|
|
||||||
|
|
||||||
For new code, please run [`clint.py`][clint] to detect style errors. It is not
|
Weigh those costs when making cosmetic changes. In general, avoid pull
|
||||||
perfect and may have false positives and negatives. To have `clint.py` ignore
|
requests dominated by style changes, but feel free to fix up lines that you
|
||||||
certain special cases, put `// NOLINT` at the end of the line.
|
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.
|
||||||
|
|
||||||
We also provide a configuration file for [`clang-format` and
|
For new code, run `make lint` (which runs [clint.py][clint]) to detect style
|
||||||
`git-clang-format`][clang-format], which can be used to format code according
|
errors. Make sure that the file(s) you intend to be linted are not in
|
||||||
to the style guidelines. Be aware this formatting method might need user
|
`clint-ignored-files.txt`. It's not perfect, so some warnings may be false
|
||||||
supervision.
|
positives/negatives. To have `clint.py` ignore certain cases, put `// NOLINT`
|
||||||
|
at the end of the line.
|
||||||
|
|
||||||
#### Commit guidelines
|
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:
|
||||||
|
|
||||||
The purpose of these guidelines is to *make reviews easier* and make the VCS logs more valuable.
|
```c
|
||||||
|
int formatted_code;
|
||||||
|
// clang-format off
|
||||||
|
void unformatted_code ;
|
||||||
|
// clang-format on
|
||||||
|
void formatted_code_again;
|
||||||
|
```
|
||||||
|
|
||||||
- Try to keep the first line under 70 characters.
|
### Commit guidelines
|
||||||
- Include further description, if necessary, after a blank line.
|
|
||||||
- Don't make it too verbose by including obvious things.
|
The purpose of these guidelines is to *make reviews easier* and make the
|
||||||
- But don't spare clarifications for anything that could be not so obvious.
|
[VCS][vcs] logs more valuable.
|
||||||
Some commit messages are pages long, and that's fine if there's no better
|
|
||||||
place for those comments to live.
|
- 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
|
- **Recommended:** Prefix logically-related commits with a consistent
|
||||||
identifier at the beginning of each commit message.
|
identifier in each commit message.
|
||||||
[For example](https://github.com/neovim/neovim/commits?author=elmart),
|
[For example](https://github.com/neovim/neovim/commits?author=elmart),
|
||||||
the following commits are related by task (*Introduce vim namespace*) and
|
the following commits are related by task (*Introduce nvim namespace*) and
|
||||||
scope (*Contrib YCM*).
|
scope (*Contrib YCM*).
|
||||||
<br/> `Introduce vim namespace: Contrib YCM: Fix style issues.`
|
<br/> `Introduce nvim namespace: Contrib YCM: Fix style issues`
|
||||||
<br/> `Introduce vim namespace: Contrib YCM: Fix build dir calculation`
|
<br/> `Introduce nvim namespace: Contrib YCM: Fix build dir calculation`
|
||||||
- Subtasks can be *activity-oriented* (doing different things on the same area)
|
- Subtasks can be *activity-oriented* (doing different things on the same area)
|
||||||
or *scope-oriented* (doing the same thing on different areas).
|
or *scope-oriented* (doing the same thing on different areas).
|
||||||
- Granularity helps, but it's conceptual size that matters, not extent size.
|
- Granularity helps, but it's conceptual size that matters, not extent size.
|
||||||
- Use the imperative voice: "Fix bug" rather than "Fixed bug" or "Fixes bug."
|
- 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.
|
||||||
|
|
||||||
|
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]:
|
||||||
|
|
||||||
|
hub checkout https://github.com/neovim/neovim/pull/1820
|
||||||
|
|
||||||
|
Use [`git log -p master..FETCH_HEAD`][git-history-filtering] to list all
|
||||||
|
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].
|
||||||
|
|
||||||
[cla]: https://docs.google.com/forms/d/1u54bpbwzneDIRltFx1TGi2evKxY3w0cOV3vlpj8DPbg/viewform
|
[cla]: https://docs.google.com/forms/d/1u54bpbwzneDIRltFx1TGi2evKxY3w0cOV3vlpj8DPbg/viewform
|
||||||
[clint]: clint.py
|
|
||||||
[clang-format]: http://clang.llvm.org/docs/ClangFormat.html
|
[clang-format]: http://clang.llvm.org/docs/ClangFormat.html
|
||||||
[entry]: https://github.com/neovim/neovim/issues?labels=entry-level&state=open
|
[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
|
||||||
|
[git-feature-branch]: https://www.atlassian.com/git/tutorials/comparing-workflows
|
||||||
|
[git-history-filtering]: https://www.atlassian.com/git/tutorials/git-log/filtering-the-commit-history
|
||||||
|
[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
|
||||||
|
[hub]: https://hub.github.com/
|
||||||
[imperative]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
|
[imperative]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
|
||||||
[style]: http://neovim.org/develop/style-guide.xml
|
[style-guide]: http://neovim.org/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
|
[waffle]: https://waffle.io/neovim/neovim
|
||||||
[wiki]: https://github.com/neovim/neovim/wiki/Contributing
|
[wiki-building-testing]: https://github.com/neovim/neovim/wiki/Building-Neovim#testing
|
||||||
|
[wiki-building-troubleshooting]: https://github.com/neovim/neovim/wiki/Building-Neovim#troubleshootingfaq
|
||||||
|
[wiki-contributing]: https://github.com/neovim/neovim/wiki/Contributing
|
||||||
|
[wiki-install-troubleshooting]: https://github.com/neovim/neovim/wiki/Installing-Neovim#troubleshooting
|
||||||
|
[wiki-review-checklist]: https://github.com/neovim/neovim/wiki/Code-review-checklist
|
||||||
|
Loading…
Reference in New Issue
Block a user