diff options
Diffstat (limited to 'CONTRIBUTING.md')
| -rw-r--r-- | CONTRIBUTING.md | 82 |
1 files changed, 56 insertions, 26 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7389ece75..20bce8ca62 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,10 +6,16 @@ Getting started If you want to help but don't know where to start, here are some low-risk/isolated tasks: -- Help us [review pull requests](#reviewing)! -- Merge a [Vim patch]. +- [Merge a Vim patch]. - Try a [complexity:low] issue. -- Fix [clang-scan] or [coverity](#coverity) warnings. +- Fix bugs found by [clang scan-build](#clang-scan-build), + [coverity](#coverity), and [PVS](#pvs-studio). + +Developer guidelines +-------------------- + +- Nvim developers should read `:help dev-help`. +- External UI developers should read `:help dev-ui`. Reporting problems ------------------ @@ -17,26 +23,33 @@ Reporting problems - 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. +- Disable plugins incrementally, to narrow down the cause of the issue. +- When reporting a crash, include a stacktrace. +- [Bisect][git-bisect] to the cause of a regression, if you are able. This is _extremely_ helpful. +- Check `$NVIM_LOG_FILE`, if it exists. +- Include `cmake --system-information` for **build** issues. 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. +- To avoid duplicate work, create a `[WIP]` pull request as soon as possible. +- Avoid cosmetic changes to unrelated files in the same commit. - 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. +- Use a **rebase workflow** for small PRs. + - After addressing review comments, it's fine to rebase and force-push. +- Use a **merge workflow** for big, high-risk PRs. + - Merge `master` into your PR when there are conflicts or when master + introduces breaking changes. + - Use the `ri` git alias: + ``` + [alias] + ri = "!sh -c 't=\"${1:-master}\"; s=\"${2:-HEAD}\"; mb=\"$(git merge-base \"$t\" \"$s\")\"; if test \"x$mb\" = x ; then o=\"$t\"; else lm=\"$(git log -n1 --merges \"$t..$s\" --pretty=%H)\"; if test \"x$lm\" = x ; then o=\"$mb\"; else o=\"$lm\"; fi; fi; test $# -gt 0 && shift; test $# -gt 0 && shift; git rebase --interactive \"$o\" \"$@\"'" + ``` + This avoids unnecessary rebases yet still allows you to combine related + commits, separate monolithic commits, etc. + - Do not edit commits that come before the merge commit. +- During a squash/fixup, use `exec make -C build unittest` between each + pick/edit/reword. ### Stages: WIP, RFC, RDY @@ -73,16 +86,18 @@ the VCS/git logs more valuable. ### Automated builds (CI) -Each pull request must pass the automated builds ([travis CI] and [quickbuild]). +Each pull request must pass the automated builds on [travis CI], [quickbuild] +and [AppVeyor]. -- CI builds are compiled with [`-Werror`][gcc-warnings], so if your PR - introduces any compiler warnings, the build will fail. +- CI builds are compiled with [`-Werror`][gcc-warnings], so compiler warnings + will fail the build. - 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` +- CI runs [ASan] and other analyzers. + - To run valgrind locally: `VALGRIND=1 make test` + - To run Clang ASan/UBSan locally: `CC=clang make CMAKE_FLAGS="-DCLANG_ASAN_UBSAN=ON"` - 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. @@ -99,11 +114,19 @@ QuickBuild uses this invocation: VERBOSE=1 nvim unittest-prereqs functionaltest-prereqs +### Clang scan-build + +The auto-generated [clang-scan] report presents walk-throughs of bugs found by +Clang's [scan-build](https://clang-analyzer.llvm.org/scan-build.html) static +analyzer. To verify a fix locally, run `scan-build` like this: + + rm -rf build/ + scan-build --use-analyzer=/usr/bin/clang make + ### Coverity [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. +master build. To view the defects, just request access; you will be approved. Use this commit-message format for coverity fixes: @@ -111,6 +134,12 @@ Use this commit-message format for coverity fixes: where `<id>` is the Coverity ID (CID). For example see [#804](https://github.com/neovim/neovim/pull/804). +### PVS-Studio + +View the [PVS analysis report](https://neovim.io/doc/reports/pvs/) to see bugs +found by [PVS Studio](https://www.viva64.com/en/pvs-studio/). +You can run `scripts/pvscheck.sh` locally to run PVS on your machine. + Reviewing --------- @@ -145,6 +174,7 @@ as context, use the `-W` argument as well. [3174]: https://github.com/neovim/neovim/issues/3174 [travis CI]: https://travis-ci.org/neovim/neovim [quickbuild]: http://neovim-qb.szakmeister.net/dashboard -[Vim patch]: https://github.com/neovim/neovim/wiki/Merging-patches-from-upstream-Vim +[AppVeyor]: https://ci.appveyor.com/project/neovim/neovim +[Merge a Vim patch]: https://github.com/neovim/neovim/wiki/Merging-patches-from-upstream-Vim [clang-scan]: https://neovim.io/doc/reports/clang/ [complexity:low]: https://github.com/neovim/neovim/issues?q=is%3Aopen+is%3Aissue+label%3Acomplexity%3Alow |