On 02/10/2016 22:47, Michael Niedermayer wrote:
On Sun, Oct 02, 2016 at 01:51:41AM +0100, Josh de Kock wrote:
Explicitly state that FATE should pass, and code should work
for all reviewers who tested.
Signed-off-by: Josh de Kock <j...@itanimul.li>
---
doc/developer.texi | 91 ++++++++++++++++++++++++++----------------------------
1 file changed, 44 insertions(+), 47 deletions(-)
diff --git a/doc/developer.texi b/doc/developer.texi
index 4d3a7ae..0075a27 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -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.
"You can" and "may be permitted under-circumstances" has rather
different meaning. Also the later is bad in a text like this
as its ambigous ...
-@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
-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 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.
+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 breakages and changes should be discussed before they are made.
I dont think this should list "breakages"
"breakages" are a subset of "changes"
and except in exteemly rare cases "breakages" should not happen
intentionally
That makes sense, I'll push a couple days with `s/breakages and //` if
there are no further comments.
Thanks for the review.
--
Josh
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
