[FFmpeg-devel] [PATCH v2 1/2] doc/developer: reword some of the policies

[Michael Niedermayer]

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 <josh at 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
> - at item
> + at 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.
>  
> - at 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.
> + at 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 ...


>  
> - at item
> + at 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.
>  
> - at 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.
> -
> - at 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.
> + at 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.
> +
> + at 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.
>  

> - at item
> + at 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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161002/9ce3cda0/attachment.sig>