Message ID | 20161002005142.21046-1-josh@itanimul.li |
---|---|
State | Accepted |
Headers | show |
On 10/1/2016 9:51 PM, 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@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. > > -@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. > 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! > - > -Note: Redundant code can be removed. > +Do not remove widely used functionality or features (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. > Can't find anything wrong. The additions seem fine, so LGTM. In any case, wait a bit for someone else to comment as well.
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@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 [...]
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 <josh@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.
On Sun, Oct 02, 2016 at 11:16:49PM +0100, Josh de Kock wrote: > > > 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. [...] > >>-@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. not sure this fits in the patchset but maybe "deprecation" should be added as deprecation implies future change. Such change of course needs to be discussed. Discussing it at the time of deprecation is better than after. [...]
On 03/10/2016 00:05, Michael Niedermayer wrote: > On Sun, Oct 02, 2016 at 11:16:49PM +0100, Josh de Kock wrote: >> >> >> 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. > [...] >>>> -@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. > > not sure this fits in the patchset but > maybe "deprecation" should be added as deprecation implies future > change. Such change of course needs to be discussed. Discussing it > at the time of deprecation is better than after. > Applied without the deprecation addition.
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. -@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. 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! - -Note: Redundant code can be removed. +Do not remove widely used functionality or features (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.
Explicitly state that FATE should pass, and code should work for all reviewers who tested. Signed-off-by: Josh de Kock <josh@itanimul.li> --- doc/developer.texi | 91 ++++++++++++++++++++++++++---------------------------- 1 file changed, 44 insertions(+), 47 deletions(-)