mirror of https://git.ffmpeg.org/ffmpeg.git
doc/developer: reword some of the policies
Explicitly state that FATE should pass, and code should work for all reviewers who tested. Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Josh de Kock <josh@itanimul.li>
This commit is contained in:
parent
21d3f0c020
commit
36fa3d8807
|
@ -247,7 +247,7 @@ For Emacs, add these roughly equivalent lines to your @file{.emacs.d/init.el}:
|
|||
@section Development Policy
|
||||
|
||||
@enumerate
|
||||
@item
|
||||
@item Licenses for patches must be compatible with FFmpeg.
|
||||
Contributions should be licensed under the
|
||||
@uref{http://www.gnu.org/licenses/lgpl-2.1.html, LGPL 2.1},
|
||||
including an "or any later version" clause, or, if you prefer
|
||||
|
@ -260,15 +260,15 @@ preferred.
|
|||
If you add a new file, give it a proper license header. Do not copy and
|
||||
paste it from a random place, use an existing file as template.
|
||||
|
||||
@item
|
||||
You must not commit code which breaks FFmpeg! (Meaning unfinished but
|
||||
enabled code which breaks compilation or compiles but does not work or
|
||||
breaks the regression tests)
|
||||
You can commit unfinished stuff (for testing etc), but it must be disabled
|
||||
(#ifdef etc) by default so it does not interfere with other developers'
|
||||
work.
|
||||
@item You must not commit code which breaks FFmpeg!
|
||||
This means unfinished code which is enabled and breaks compilation,
|
||||
or compiles but does not work/breaks the regression tests. Code which
|
||||
is unfinished but disabled may be permitted under-circumstances, like
|
||||
missing samples or an implementation with a small subset of features.
|
||||
Always check the mailing list for any reviewers with issues and test
|
||||
FATE before you push.
|
||||
|
||||
@item
|
||||
@item Keep the main commit message short with an extended description below.
|
||||
The commit message should have a short first line in the form of
|
||||
a @samp{topic: short description} as a header, separated by a newline
|
||||
from the body consisting of an explanation of why the change is necessary.
|
||||
|
@ -276,30 +276,29 @@ If the commit fixes a known bug on the bug tracker, the commit message
|
|||
should include its bug ID. Referring to the issue on the bug tracker does
|
||||
not exempt you from writing an excerpt of the bug in the commit message.
|
||||
|
||||
@item
|
||||
You do not have to over-test things. If it works for you, and you think it
|
||||
should work for others, then commit. If your code has problems
|
||||
(portability, triggers compiler bugs, unusual environment etc) they will be
|
||||
reported and eventually fixed.
|
||||
@item Testing must be adequate but not excessive.
|
||||
If it works for you, others, and passes FATE then it should be OK to commit
|
||||
it, provided it fits the other committing criteria. You should not worry about
|
||||
over-testing things. If your code has problems (portability, triggers
|
||||
compiler bugs, unusual environment etc) they will be reported and eventually
|
||||
fixed.
|
||||
|
||||
@item
|
||||
Do not commit unrelated changes together, split them into self-contained
|
||||
pieces. Also do not forget that if part B depends on part A, but A does not
|
||||
depend on B, then A can and should be committed first and separate from B.
|
||||
Keeping changes well split into self-contained parts makes reviewing and
|
||||
understanding them on the commit log mailing list easier. This also helps
|
||||
in case of debugging later on.
|
||||
@item Do not commit unrelated changes together.
|
||||
They should be split them into self-contained pieces. Also do not forget
|
||||
that if part B depends on part A, but A does not depend on B, then A can
|
||||
and should be committed first and separate from B. Keeping changes well
|
||||
split into self-contained parts makes reviewing and understanding them on
|
||||
the commit log mailing list easier. This also helps in case of debugging
|
||||
later on.
|
||||
Also if you have doubts about splitting or not splitting, do not hesitate to
|
||||
ask/discuss it on the developer mailing list.
|
||||
|
||||
@item
|
||||
@item API/ABI changes should be discussed before they are made.
|
||||
Do not change behavior of the programs (renaming options etc) or public
|
||||
API or ABI without first discussing it on the ffmpeg-devel mailing list.
|
||||
Do not remove functionality from the code. Just improve!
|
||||
Do not remove widely used functionality or features (redundant code can be removed).
|
||||
|
||||
Note: Redundant code can be removed.
|
||||
|
||||
@item
|
||||
@item Ask before you change the build system (configure, etc).
|
||||
Do not commit changes to the build system (Makefiles, configure script)
|
||||
which change behavior, defaults etc, without asking first. The same
|
||||
applies to compiler warning fixes, trivial looking fixes and to code
|
||||
|
@ -308,7 +307,7 @@ the way we do. Send your changes as patches to the ffmpeg-devel mailing
|
|||
list, and if the code maintainers say OK, you may commit. This does not
|
||||
apply to files you wrote and/or maintain.
|
||||
|
||||
@item
|
||||
@item Cosmetic changes should be kept in separate patches.
|
||||
We refuse source indentation and other cosmetic changes if they are mixed
|
||||
with functional changes, such commits will be rejected and removed. Every
|
||||
developer has his own indentation style, you should not change it. Of course
|
||||
|
@ -322,7 +321,7 @@ NOTE: If you had to put if()@{ .. @} over a large (> 5 lines) chunk of code,
|
|||
then either do NOT change the indentation of the inner part within (do not
|
||||
move it to the right)! or do so in a separate commit
|
||||
|
||||
@item
|
||||
@item Commit messages should always be filled out properly.
|
||||
Always fill out the commit log message. Describe in a few lines what you
|
||||
changed and why. You can refer to mailing list postings if you fix a
|
||||
particular bug. Comments such as "fixed!" or "Changed it." are unacceptable.
|
||||
|
@ -334,35 +333,35 @@ area changed: Short 1 line description
|
|||
details describing what and why and giving references.
|
||||
@end example
|
||||
|
||||
@item
|
||||
@item Credit the author of the patch.
|
||||
Make sure the author of the commit is set correctly. (see git commit --author)
|
||||
If you apply a patch, send an
|
||||
answer to ffmpeg-devel (or wherever you got the patch from) saying that
|
||||
you applied the patch.
|
||||
|
||||
@item
|
||||
@item Complex patches should refer to discussion surrounding them.
|
||||
When applying patches that have been discussed (at length) on the mailing
|
||||
list, reference the thread in the log message.
|
||||
|
||||
@item
|
||||
@item Always wait long enough before pushing changes
|
||||
Do NOT commit to code actively maintained by others without permission.
|
||||
Send a patch to ffmpeg-devel instead. If no one answers within a reasonable
|
||||
timeframe (12h for build failures and security fixes, 3 days small changes,
|
||||
Send a patch to ffmpeg-devel. If no one answers within a reasonable
|
||||
time-frame (12h for build failures and security fixes, 3 days small changes,
|
||||
1 week for big patches) then commit your patch if you think it is OK.
|
||||
Also note, the maintainer can simply ask for more time to review!
|
||||
|
||||
@item
|
||||
Subscribe to the ffmpeg-cvslog mailing list. The diffs of all commits
|
||||
are sent there and reviewed by all the other developers. Bugs and possible
|
||||
improvements or general questions regarding commits are discussed there. We
|
||||
expect you to react if problems with your code are uncovered.
|
||||
@item Subscribe to the ffmpeg-cvslog mailing list.
|
||||
It is important to do this as the diffs of all commits are sent there and
|
||||
reviewed by all the other developers. Bugs and possible improvements or
|
||||
general questions regarding commits are discussed there. We expect you to
|
||||
react if problems with your code are uncovered.
|
||||
|
||||
@item
|
||||
@item Keep the documentation up to date.
|
||||
Update the documentation if you change behavior or add features. If you are
|
||||
unsure how best to do this, send a patch to ffmpeg-devel, the documentation
|
||||
maintainer(s) will review and commit your stuff.
|
||||
|
||||
@item
|
||||
@item Important discussions should be accessible to all.
|
||||
Try to keep important discussions and requests (also) on the public
|
||||
developer mailing list, so that all developers can benefit from them.
|
||||
|
||||
|
@ -371,10 +370,8 @@ Never write to unallocated memory, never write over the end of arrays,
|
|||
always check values read from some untrusted source before using them
|
||||
as array index or other risky things.
|
||||
|
||||
@item
|
||||
Remember to check if you need to bump versions for the specific libav*
|
||||
parts (libavutil, libavcodec, libavformat) you are changing. You need
|
||||
to change the version integer.
|
||||
@item Remember to check if you need to bump versions for libav*.
|
||||
Depending on the change, you may need to change the version integer.
|
||||
Incrementing the first component means no backward compatibility to
|
||||
previous versions (e.g. removal of a function from the public API).
|
||||
Incrementing the second component means backward compatible change
|
||||
|
@ -384,7 +381,7 @@ Incrementing the third component means a noteworthy binary compatible
|
|||
change (e.g. encoder bug fix that matters for the decoder). The third
|
||||
component always starts at 100 to distinguish FFmpeg from Libav.
|
||||
|
||||
@item
|
||||
@item Warnings for correct code may be disabled if there is no other option.
|
||||
Compiler warnings indicate potential bugs or code with bad style. If a type of
|
||||
warning always points to correct and clean code, that warning should
|
||||
be disabled, not the code changed.
|
||||
|
@ -393,7 +390,7 @@ If it is a bug, the bug has to be fixed. If it is not, the code should
|
|||
be changed to not generate a warning unless that causes a slowdown
|
||||
or obfuscates the code.
|
||||
|
||||
@item
|
||||
@item Check your entries in MAINTAINERS.
|
||||
Make sure that no parts of the codebase that you maintain are missing from the
|
||||
@file{MAINTAINERS} file. If something that you want to maintain is missing add it with
|
||||
your name after it.
|
||||
|
|
Loading…
Reference in New Issue