2014-06-17 21:15:25 +00:00
|
|
|
How to contribute
|
|
|
|
=================
|
2014-03-12 15:37:54 +00:00
|
|
|
|
2014-06-17 21:15:25 +00:00
|
|
|
General
|
|
|
|
-------
|
|
|
|
|
2015-04-27 21:02:40 +00:00
|
|
|
The main contact for mpv development is IRC, specifically #mpv
|
2021-06-08 14:34:01 +00:00
|
|
|
and #mpv-devel on Libera.chat. Github is used for code review and
|
2019-10-01 20:39:56 +00:00
|
|
|
long term discussions.
|
2014-06-17 21:15:25 +00:00
|
|
|
|
|
|
|
Sending patches
|
|
|
|
---------------
|
|
|
|
|
|
|
|
- Make a github pull request, or send a link to a plaintext patch created with
|
2019-10-01 20:39:56 +00:00
|
|
|
``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
|
|
|
|
commit message and authorship information.
|
|
|
|
- Never send patches to any of the developers email addresses.
|
|
|
|
- If your changes are not supposed to be merged immediately, mark them as
|
|
|
|
"[RFC]" in the commit message or the pull request title.
|
|
|
|
- Be sure to test your changes. If you didn't, please say so in the commit
|
|
|
|
message and the pull request text.
|
|
|
|
|
|
|
|
Copyright of contributions
|
|
|
|
--------------------------
|
|
|
|
|
|
|
|
- The copyright belongs to contributors. The project is a collaborative work. By
|
|
|
|
sending your changes, you agree to license your contributions according to the
|
|
|
|
requirements of this project.
|
2015-12-22 22:09:18 +00:00
|
|
|
- All new code must be LGPLv2.1+ licensed, or come with the implicit agreement
|
|
|
|
that it will be relicensed to LGPLv2.1+ later (see ``Copyright`` in the
|
|
|
|
repository root directory).
|
2019-10-01 20:39:56 +00:00
|
|
|
- 100% compatible licenses are allowed too.
|
|
|
|
- Changes in files with more liberal licenses (such as BSD, MIT, or ISC) are
|
2019-09-21 11:58:09 +00:00
|
|
|
assumed to be dual-licensed under LGPLv2.1+ and the license indicated in the
|
|
|
|
file header.
|
2017-06-24 19:02:45 +00:00
|
|
|
- You must be either the exclusive author of the patch, or acknowledge all
|
|
|
|
authors involved in the commit message. If you take 3rd party code, authorship
|
2019-10-01 20:39:56 +00:00
|
|
|
and copyright must be properly acknowledged. If you're making changes on
|
|
|
|
behalf of your employer, and the employer owns the copyright, you must mention
|
|
|
|
this. If the license of the code is not LGPLv2.1+, you must mention this.
|
2019-09-21 11:58:09 +00:00
|
|
|
- These license statements are legally binding.
|
2019-11-07 20:02:12 +00:00
|
|
|
- Don't use fake names (something that looks like an actual name, and may be
|
2017-06-24 19:02:45 +00:00
|
|
|
someone else's name, but is not your legal name). Using a pseudonyms is
|
|
|
|
allowed if it can be used to identify or contact you, even if whatever
|
|
|
|
account you used to submit the patch dies.
|
2019-10-01 20:39:56 +00:00
|
|
|
- Do not add your name to the license header. This convention is not used by
|
|
|
|
this project, and neither copyright law nor any of the used licenses require
|
|
|
|
it.
|
|
|
|
|
|
|
|
Write good commit messages
|
|
|
|
--------------------------
|
|
|
|
|
2014-06-17 21:15:25 +00:00
|
|
|
- Write informative commit messages. Use present tense to describe the
|
|
|
|
situation with the patch applied, and past tense for the situation before
|
|
|
|
the change.
|
2019-12-15 13:58:51 +00:00
|
|
|
- The subject line (the first line in a commit message) must contain a
|
2014-06-17 21:15:25 +00:00
|
|
|
prefix identifying the sub system, followed by a short description what
|
2017-06-13 20:42:12 +00:00
|
|
|
impact this commit has. This subject line and the commit message body
|
2019-12-15 13:58:51 +00:00
|
|
|
must not be longer than 72 characters per line, because it messes up the
|
2017-06-13 20:42:12 +00:00
|
|
|
output of many git tools otherwise.
|
2014-06-17 21:15:25 +00:00
|
|
|
|
|
|
|
For example, you fixed a crash in af_volume.c:
|
|
|
|
|
2015-12-22 22:09:18 +00:00
|
|
|
- Bad: ``fixed the bug (wtf?)``
|
|
|
|
- Good: ``af_volume: fix crash due to null pointer access``
|
2014-06-17 21:15:25 +00:00
|
|
|
|
|
|
|
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``.
|
2021-08-15 19:15:27 +00:00
|
|
|
|
|
|
|
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.
|
|
|
|
|
2019-11-18 17:14:07 +00:00
|
|
|
- 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.
|
2019-12-15 13:58:51 +00:00
|
|
|
- The body of the commit message (everything else after the subject line) must
|
2014-06-17 21:15:25 +00:00
|
|
|
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
|
|
|
|
later, and finds that it's caused by your commit (even though your commit was
|
|
|
|
supposed to fix another bug), it would be bad if there wasn't enough
|
|
|
|
information to test the original bug. The old bug might be reintroduced while
|
|
|
|
fixing the new bug.
|
|
|
|
|
2019-12-15 13:58:51 +00:00
|
|
|
The commit message must be wrapped on 72 characters per line, because git
|
2014-06-17 21:15:25 +00:00
|
|
|
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).
|
2019-10-01 20:39:56 +00:00
|
|
|
- Another summary of good conventions: https://chris.beams.io/posts/git-commit/
|
|
|
|
|
|
|
|
Split changes into multiple commits
|
|
|
|
-----------------------------------
|
|
|
|
|
|
|
|
- Follow git good practices, and split independent changes into several commits.
|
|
|
|
It's usually OK to put them into a single pull request.
|
2014-06-17 21:15:25 +00:00
|
|
|
- Try to separate cosmetic and functional changes. It's ok to make a few
|
|
|
|
additional cosmetic changes in the same file you're working on. But don't do
|
|
|
|
something like reformatting a whole file, and hiding an actual functional
|
|
|
|
change in the same commit.
|
2019-11-18 17:14:07 +00:00
|
|
|
- Splitting changes does _not_ mean that you should make them as fine-grained
|
|
|
|
as possible. Commits should form logical steps in development. The way you
|
|
|
|
split changes is important for code review and analyzing bugs.
|
2019-10-01 20:39:56 +00:00
|
|
|
|
2020-04-30 23:13:20 +00:00
|
|
|
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).
|
|
|
|
|
2019-10-01 20:39:56 +00:00
|
|
|
Touching user-visible parts may require updating the mpv docs
|
|
|
|
-------------------------------------------------------------
|
|
|
|
|
|
|
|
- Most user-visible things are normally documented in DOCS/man/. If your commit
|
|
|
|
touches documented behavior, list of sub-options, etc., you need to adjust the
|
|
|
|
documentation.
|
|
|
|
- These changes usually go into the same commit that changes the code.
|
2017-06-24 19:02:45 +00:00
|
|
|
- Changes to command line options (addition/modification/removal) must be
|
2019-10-01 20:39:56 +00:00
|
|
|
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.
|
2014-03-12 15:37:54 +00:00
|
|
|
|
2014-06-17 21:15:25 +00:00
|
|
|
Code formatting
|
|
|
|
---------------
|
|
|
|
|
|
|
|
mpv uses C99 with K&R formatting, with some exceptions.
|
2014-03-12 15:37:54 +00:00
|
|
|
|
|
|
|
- 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
|
2019-12-15 13:58:51 +00:00
|
|
|
cases where spaces must be added. Example:
|
2014-03-12 15:37:54 +00:00
|
|
|
|
|
|
|
```C
|
|
|
|
if ((a * b) > c) {
|
|
|
|
// code
|
|
|
|
some_function(a, b, c);
|
|
|
|
}
|
|
|
|
```
|
|
|
|
- Break lines on 80 columns. There is a hard limit of 85 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
|
|
|
|
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.
|
|
|
|
|
|
|
|
Bad:
|
|
|
|
|
|
|
|
```C
|
|
|
|
if (a)
|
|
|
|
// do something if b
|
|
|
|
if (b)
|
|
|
|
do_something();
|
|
|
|
```
|
|
|
|
|
|
|
|
Good:
|
|
|
|
|
|
|
|
```C
|
|
|
|
if (a) {
|
|
|
|
// do something if b
|
|
|
|
if (b)
|
|
|
|
do_something();
|
|
|
|
}
|
|
|
|
```
|
2019-12-15 13:58:51 +00:00
|
|
|
- If the if has an else branch, both branches must use braces, even if they're
|
2017-02-11 15:48:29 +00:00
|
|
|
technically redundant.
|
2014-03-12 23:42:50 +00:00
|
|
|
|
|
|
|
Example:
|
|
|
|
|
|
|
|
```C
|
|
|
|
if (a) {
|
|
|
|
one_line();
|
2017-02-11 15:48:29 +00:00
|
|
|
} else {
|
|
|
|
one_other_line();
|
2014-03-12 23:42:50 +00:00
|
|
|
}
|
|
|
|
```
|
|
|
|
- If an if condition spans multiple physical lines, then put the opening brace
|
|
|
|
for the if body on the next physical line. (Also, preferably always add a
|
|
|
|
brace, even if technically none is needed.)
|
|
|
|
|
|
|
|
Example:
|
|
|
|
|
|
|
|
```C
|
|
|
|
if (very_long_condition_a &&
|
|
|
|
very_long_condition_b)
|
|
|
|
{
|
|
|
|
code();
|
2015-03-08 14:51:47 +00:00
|
|
|
} else {
|
|
|
|
...
|
2014-03-12 23:42:50 +00:00
|
|
|
}
|
|
|
|
```
|
2017-08-30 08:34:16 +00:00
|
|
|
|
|
|
|
(If the if body is simple enough, this rule can be skipped.)
|
2014-03-12 15:37:54 +00:00
|
|
|
- Remove any trailing whitespace.
|
2019-10-01 20:39:56 +00:00
|
|
|
- Do not make stray whitespaces changes.
|
2014-03-12 15:37:54 +00:00
|
|
|
|
2019-11-07 20:02:58 +00:00
|
|
|
Header #include statement order
|
|
|
|
-------------------------------
|
|
|
|
|
|
|
|
The order of ``#include`` statements in the source code is not very consistent.
|
2019-12-15 13:58:51 +00:00
|
|
|
New code must follow the following conventions:
|
2019-11-07 20:02:58 +00:00
|
|
|
|
|
|
|
- Put standard includes (``#include <stdlib.h>`` etc.) on the top,
|
2019-12-15 15:33:58 +00:00
|
|
|
- then after a blank line, add library includes (``#include <zlib.h>`` etc.)
|
2019-11-07 20:02:58 +00:00
|
|
|
- then after a blank line, add internal includes (``#include "player/core.h"``)
|
|
|
|
- sort them alphabetically within these sections
|
|
|
|
|
2014-03-12 15:37:54 +00:00
|
|
|
General coding
|
|
|
|
--------------
|
|
|
|
|
|
|
|
- Use C99. Also freely make use of C99 features if it's appropriate, such as
|
2019-09-21 14:35:45 +00:00
|
|
|
stdbool.h. (Except VLA and complex number types.)
|
2019-09-26 12:12:03 +00:00
|
|
|
- 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.
|
2019-12-15 13:58:51 +00:00
|
|
|
But in general, standard C99 must be used.
|
2014-03-12 15:37:54 +00:00
|
|
|
- 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
|
2019-09-26 12:12:03 +00:00
|
|
|
configure check. There is some restricted use of C11 (ask on IRC for details).
|
2014-03-12 15:37:54 +00:00
|
|
|
- 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.
|
|
|
|
- If you add features that require intrusive changes, discuss them on the dev
|
|
|
|
channel first. There might be a better way to add a feature and it can avoid
|
|
|
|
wasted work.
|
2017-06-24 19:03:29 +00:00
|
|
|
|
2019-10-01 20:39:56 +00:00
|
|
|
Code of Conduct
|
|
|
|
---------------
|
|
|
|
|
2020-08-12 15:32:02 +00:00
|
|
|
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/
|
2019-10-01 20:39:56 +00:00
|
|
|
|
2017-06-24 19:03:29 +00:00
|
|
|
Rules for git push access
|
|
|
|
-------------------------
|
|
|
|
|
|
|
|
Push access to the main git repository is handed out on an arbitrary basis. If
|
|
|
|
you got access, the following rules must be followed:
|
|
|
|
|
|
|
|
- You are expected to follow the general development rules as outlined in this
|
|
|
|
whole document.
|
|
|
|
- You must be present on the IRC dev channel when you push something.
|
|
|
|
- Anyone can push small fixes: typo corrections, small/obvious/uncontroversial
|
|
|
|
bug fixes, edits to the user documentation or code comments, and so on.
|
|
|
|
- You can freely make changes to parts of the code which you maintain. For
|
|
|
|
larger changes, it's recommended to let others review the changes first.
|
|
|
|
- You automatically maintain code if you wrote or modified most of it before
|
|
|
|
(e.g. you made larger changes to it before, did partial or full rewrites, did
|
|
|
|
major bug fixes, or you're the original author of the code). If there is more
|
|
|
|
than one maintainer, you may need to come to an agreement with the others how
|
|
|
|
to handle this to avoid conflict.
|
2019-10-01 20:39:56 +00:00
|
|
|
- If you make a pull requests (especially if it's to code you maintain), and you
|
|
|
|
want reviews, explicitly ping the people from which you expect reviews.
|
|
|
|
- As a maintainer, you can approve pull requests by others to "your" code.
|
2017-06-24 19:03:29 +00:00
|
|
|
- If you approve or merge 3rd party changes, make sure they follow the general
|
|
|
|
development rules.
|
|
|
|
- Changes to user interface and public API must always be approved by the
|
|
|
|
project leader.
|
|
|
|
- Seasoned project members are allowed to revert commits that broke the build,
|
|
|
|
or broke basic functionality in a catastrophic way, and the developer who
|
|
|
|
broke it is unavailable. (Depending on severity.)
|
|
|
|
- Adhere to the CoC.
|
|
|
|
- The project leader is not bound by these rules.
|