diff options
Diffstat (limited to 'DOCS/contribute.md')
| -rw-r--r-- | DOCS/contribute.md | 81 |
1 files changed, 59 insertions, 22 deletions
diff --git a/DOCS/contribute.md b/DOCS/contribute.md index 319968bd83..4d2af4ea4e 100644 --- a/DOCS/contribute.md +++ b/DOCS/contribute.md @@ -5,13 +5,13 @@ General ------- The main contact for mpv development is IRC, specifically #mpv -and #mpv-devel on Freenode. Github is used for code review and +and #mpv-devel on Libera.chat. GitHub is used for code review and long term discussions. Sending patches --------------- -- Make a github pull request, or send a link to a plaintext patch created with +- Make a GitHub pull request, or send a link to a plaintext patch created with ``git format-patch``. - Plain diffs posted as pastebins are not acceptable! (Especially if the http link returns HTML.) They only cause extra work for everyone, because they lack @@ -55,10 +55,10 @@ Write good commit messages - Write informative commit messages. Use present tense to describe the situation with the patch applied, and past tense for the situation before the change. -- The subject line (the first line in a commit message) should contain a +- The subject line (the first line in a commit message) must contain a prefix identifying the sub system, followed by a short description what impact this commit has. This subject line and the commit message body - shouldn't be longer than 72 characters per line, because it messes up the + must not be longer than 72 characters per line, because it messes up the output of many git tools otherwise. For example, you fixed a crash in af_volume.c: @@ -68,11 +68,16 @@ Write good commit messages Having a prefix gives context, and is especially useful when trying to find a specific change by looking at the history, or when running ``git blame``. + + Sample prefixes: ``vo_gpu: ...``, ``command: ...``, ``DOCS/input: ...``, + ``TOOLS/osxbundle: ...``, ``osc.lua: ...``, etc. You can always check the git + log for commits which modify specific files to see which prefixes are used. + - The first word after the ``:`` is lower case. - Don't end the subject line with a ``.``. - Put an empty line between the subject line and the commit message. If this is missing, it will break display in common git tools. -- The body of the commit message (everything else after the subject line) should +- The body of the commit message (everything else after the subject line) must be as informative as possible and contain everything that isn't obvious. Don't hesitate to dump as much information as you can - it doesn't cost you anything. Put some effort into it. If someone finds a bug months or years @@ -81,7 +86,7 @@ Write good commit messages information to test the original bug. The old bug might be reintroduced while fixing the new bug. - The commit message should be wrapped on 72 characters per line, because git + The commit message must be wrapped on 72 characters per line, because git tools usually do not break text automatically. On the other hand, you do not need to break text that would be unnatural to break (like data for test cases, or long URLs). @@ -100,6 +105,22 @@ Split changes into multiple commits as possible. Commits should form logical steps in development. The way you split changes is important for code review and analyzing bugs. +Always squash fixup commits when making changes to pull requests +---------------------------------------------------------------- + +- If you make fixup commits to your pull request, you should generally squash + them with "git rebase -i". We prefer to have pull requests in a merge + ready state. +- We don't squash-merge (nor do we use GitHub's feature that does this) because + pull requests with multiple commits are perfectly legitimate, and the only + thing that makes sense in non-trivial cases. +- With complex pull requests, it *may* make sense to keep them separate, but + they should be clearly marked as such. Reviewing commits is generally easier + with fixups squashed. +- Reviewers are encouraged to look at individual commits instead of GitHub's + "changes from all commits" view (which just encourages bad git and review + practices). + Touching user-visible parts may require updating the mpv docs ------------------------------------------------------------- @@ -110,21 +131,33 @@ Touching user-visible parts may require updating the mpv docs - Changes to command line options (addition/modification/removal) must be documented in options.rst. - Changes to input properties or input commands must be documented in input.rst. -- All incompatible changes to the user interface (options, properties, commands) - must be documented with a small note in interface-changes.rst. (Additions may - be documented there as well, but this isn't required.) - Changes to the libmpv API must be reflected in the libmpv's headers doxygen, and in client-api-changes.rst. +Interface change policy +----------------------- + +- All incompatible changes to the user interface (options, properties, commands) + must be documented by making a new text file with a txt extension containing a + small note in the DOCS/interface-changes directory. +- The name of the file should be brief and related to the commit that makes the + change. +- Grouping multiple related changes in the same file is also OK. Just be sure to + put each separate change on a different line. +- Documenting additions in DOCS/interface-changes is optional but encouraged. +- interface-changes.rst is never directly updated except when making new major + releases. +- See DOCS/interface-changes/example.txt for an example. + Code formatting --------------- -mpv uses C99 with K&R formatting, with some exceptions. +mpv uses C11 with K&R formatting, with some exceptions. - Use the K&R indent style. - Use 4 spaces of indentation, never use tabs (except in Makefiles). - Add a single space between keywords and binary operators. There are some other - cases where spaces should be added. Example: + cases where spaces must be added. Example: ```C if ((a * b) > c) { @@ -132,9 +165,9 @@ mpv uses C99 with K&R formatting, with some exceptions. some_function(a, b, c); } ``` -- Break lines on 80 columns. There is a hard limit of 85 columns. You may ignore +- Break lines on 80 columns. There is a hard limit of 100 columns. You may ignore this limit if there's a strong case that not breaking the line will increase - readability. Going over 85 columns might provoke endless discussions about + readability. Going over 100 columns might provoke endless discussions about whether such a limit is needed or not, so avoid it. - If the body of an if/for/while statement has more than 1 physical lines, then always add braces, even if they're technically redundant. @@ -157,7 +190,7 @@ mpv uses C99 with K&R formatting, with some exceptions. do_something(); } ``` -- If the if has an else branch, both branches should use braces, even if they're +- If the if has an else branch, both branches must use braces, even if they're technically redundant. Example: @@ -193,25 +226,26 @@ Header #include statement order ------------------------------- The order of ``#include`` statements in the source code is not very consistent. -New code should follow the following conventions: +New code must follow the following conventions: - Put standard includes (``#include <stdlib.h>`` etc.) on the top, -- then after a blank line, add library includes (``#include <zlib.h`` etc.) +- then after a blank line, add library includes (``#include <zlib.h>`` etc.) - then after a blank line, add internal includes (``#include "player/core.h"``) - sort them alphabetically within these sections General coding -------------- -- Use C99. Also freely make use of C99 features if it's appropriate, such as - stdbool.h. (Except VLA and complex number types.) +- Use C11. Also freely make use of C11 features if it's appropriate, but do not + use VLA and complex number types. - Don't use non-standard language (such as GNU C-only features). In some cases they may be warranted, if they are optional (such as attributes enabling printf-like format string checks). "#pragma once" is allowed as an exception. - But in general, standard C99 should be used. + But in general, standard C11 must be used. - The same applies to libc functions. We have to be Windows-compatible too. Use - functions guaranteed by C99 or POSIX only, unless your use is guarded by a - configure check. There is some restricted use of C11 (ask on IRC for details). + functions guaranteed by C11 or POSIX only, unless your use is guarded by a + configure check. Be mindful of MinGW-specifics since C11 support is not always + guaranteed. - Prefer fusing declaration and initialization, rather than putting declarations on the top of a block. Obvious data flow is more important than avoiding mixing declarations and statements, which is just a C90 artifact. @@ -222,7 +256,10 @@ General coding Code of Conduct --------------- -We have one, but the document describing it got lost accidentally. +Please note that this project is released with a Contributor Code of Conduct. +By participating in this project you agree to abide by its terms. +The Contributor Code of Conduct can be found here: +https://www.contributor-covenant.org/version/2/0/code_of_conduct/ Rules for git push access ------------------------- |