| Message ID | 20171121214156.9256-1-from.ffmpeg-dev@jdlh.com |
|---|---|
| State | Superseded |
| Headers | show |
2017-11-21 22:41 GMT+01:00 Jim DeLaHunt <from.ffmpeg-dev@jdlh.com>: > -@subheading 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. I am against removing this paragraph. Carl Eugen
On 2017-11-21 15:40, Carl Eugen Hoyos wrote: > 2017-11-21 22:41 GMT+01:00 Jim DeLaHunt<from.ffmpeg-dev@jdlh.com>: > >> -@subheading 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. > I am against removing this paragraph. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Thank you for the review, Carl Eugen. Two questions for you. 1. Do you expect all contributors to ffmpeg to subscribe to the ffmpeg-cvslog email list /in addition to/ the ffmpeg-devel list? Even those who only want to contribute a bug fix or two, participate in review discussion, then leave? 2. Instead of /removing/ that paragraph, are you also against replacing it with the same paragraph but centred on the ffmpeg-devel list? The patch has the following alternative: +@section Documentation/Other +@subheading Subscribe to the ffmpeg-devel mailing list. +It is important to be subscribed to the +@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} +mailing list, because any patch you contribute must be sent there +and reviewed by the other developers. They may have comments about your +contribution. We expect you see those comments, and to improve your contribution +if requested. +Also, this list is where bugs and possible improvements or +general questions regarding commits are discussed. That may be helpful +information as you write your contribution. Finally, by being a list +subscriber your contribution will be posted immediately to the list, +without the moderation hold which messages from non-subscribers experience. Thanks, —Jim DeLaHunt
2017-11-22 0:59 GMT+01:00 Jim DeLaHunt <from.ffmpeg-dev@jdlh.com>: > On 2017-11-21 15:40, Carl Eugen Hoyos wrote: >> >> 2017-11-21 22:41 GMT+01:00 Jim DeLaHunt<from.ffmpeg-dev@jdlh.com>: >> >>> -@subheading 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. >> >> I am against removing this paragraph. [...] > 2. Instead of /removing/ that paragraph, are you also against replacing it > with the same paragraph but centred on the ffmpeg-devel list? The patch has > the following alternative: > > +@section Documentation/Other > +@subheading Subscribe to the ffmpeg-devel mailing list. > +It is important to be subscribed to the > +@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} > +mailing list, because any patch you contribute must be sent there > +and reviewed by the other developers. They may have comments about your > +contribution. We expect you see those comments, and to improve your > contribution > +if requested. > +Also, this list is where bugs and possible improvements or > +general questions regarding commits are discussed. That may be helpful > +information as you write your contribution. Finally, by being a list > +subscriber your contribution will be posted immediately to the list, > +without the moderation hold which messages from non-subscribers experience. Sorry, I do not understand this paragraph / question. If the alternative may lead to another answer than above, please consider sending a complete patch. Thank you, Carl Eugen
On 2017-11-21 16:05, Carl Eugen Hoyos wrote: > 2017-11-22 0:59 GMT+01:00 Jim DeLaHunt<from.ffmpeg-dev@jdlh.com>: >> On 2017-11-21 15:40, Carl Eugen Hoyos wrote: >>> 2017-11-21 22:41 GMT+01:00 Jim DeLaHunt<from.ffmpeg-dev@jdlh.com>: >>> >>>> -@subheading 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. >>> I am against removing this paragraph. > [...] > >> 2. Instead of /removing/ that paragraph, are you also against replacing it >> with the same paragraph but centred on the ffmpeg-devel list? The patch has >> the following alternative: >> >> +@section Documentation/Other >> +@subheading Subscribe to the ffmpeg-devel mailing list. >> +It is important to be subscribed to the >> +@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} >> +mailing list, because any patch you contribute must be sent there >> +and reviewed by the other developers. They may have comments about your >> +contribution. We expect you see those comments, and to improve your >> contribution >> +if requested. >> +Also, this list is where bugs and possible improvements or >> +general questions regarding commits are discussed. That may be helpful >> +information as you write your contribution. Finally, by being a list >> +subscriber your contribution will be posted immediately to the list, >> +without the moderation hold which messages from non-subscribers experience. > Sorry, I do not understand this paragraph / question. > > If the alternative may lead to another answer than above, please consider > sending a complete patch. Carl Eugen: I believe I sent a complete patch. Part of what that patch does is replace a section "@subheading Subscribe to the ffmpeg-cvslog mailing list." with a similar section "@subheading Subscribe to the ffmpeg-devel mailing list." Git put the delete lines first, then the replacement lines. See the lines labelled, "@@ -381,12 +379,19 @@". Do you see that part of the patch? Also, > [...] I didn't see you answer my question 1, "Do you expect all contributors to ffmpeg to subscribe to the ffmpeg-cvslog email list //in addition to// the ffmpeg-devel list?" I think that question is relevant to deciding how to handle this @subheading section. > Thank you, Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Best regards, —Jim DeLaHunt
On 11/21/2017 11:40 PM, Carl Eugen Hoyos wrote:
> I am against removing this paragraph.
He specifically asked if it was required of new developers to subscribe
to this list, in a separate thread, earlier[1]. Paul explicitly said it
was *not* required[2], Timo said no discussion happens on the list[3],
and you gave a two word reply[4] that was completely devoid of actual
information.
You can see how this may have given Jim a different impression.
Please, can we not be so abrasive and inconsistent with new contributers?
It's shameful.
- Derek
[1] http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220271.html
[2] http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220441.html
[3] http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220272.html
[4] http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220392.html
2017-11-22 15:03 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/21/2017 11:40 PM, Carl Eugen Hoyos wrote: >> I am against removing this paragraph. > > He specifically asked if it was required of new developers to subscribe > to this list, in a separate thread, earlier[1]. Paul explicitly said it > was *not* required[2], Timo said no discussion happens on the list[3], > and you gave a two word reply[4] that was completely devoid of actual > information. > > You can see how this may have given Jim a different impression. > > Please, can we not be so abrasive and inconsistent with new contributers? > It's shameful. Lol, I completely agree. Please understand I am against removing the paragraph from the documentation because I believe it is a good idea if developers are subscribed to -cvslog. Carl Eugen
On 11/22/2017 11:34 PM, Carl Eugen Hoyos wrote: > Please understand I am against removing the paragraph from the > documentation because I believe it is a good idea if developers > are subscribed to -cvslog. Perhaps it can be reworded a bit to say it's encouraged for the cited reasons, but not mandatory if you just want to send a patch here and there? - Derek
2017-11-23 0:39 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/22/2017 11:34 PM, Carl Eugen Hoyos wrote: >> Please understand I am against removing the paragraph from the >> documentation because I believe it is a good idea if developers >> are subscribed to -cvslog. > > Perhaps it can be reworded a bit to say it's encouraged for the > cited reasons, but not mandatory if you just want to send a patch > here and there? If that really helps anybody, please do so! Carl Eugen
On 2017-11-22 15:40, Carl Eugen Hoyos wrote: > 2017-11-23 0:39 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: >> On 11/22/2017 11:34 PM, Carl Eugen Hoyos wrote: >>> Please understand I am against removing the paragraph from the >>> documentation because I believe it is a good idea if developers >>> are subscribed to -cvslog. Carl Eugen: Thank you for responding clearly to the policy question I asked. You think it is a good idea for "developers" are subscribed to -cvslog. Others on this list clearly are comfortable with some contributors, maybe not the maintainers but just contributors of new features and bug fixes, not being subscribed to cvs-log. We could perhaps clarify this policy, if all of you who have that authority would care to have that discussion and come to a consensus. Personally, as a new contributor, I think a policy of requiring new contributors, and those who "just want to send a patch here and there", to subscribe to -cvslog is ridiculous. The message volume on -devel is already torrential. The message volume on -cvslog would be about double that. And, from what I see in the archives, I don't understand how the traffic on -cvslog would add value beyond what I already file unread on -devel. My opinion as someone considering whether to spend effort fixing what look to me like shallow bugs in the FFmpeg documentation, or investing my effort in another project, is that requiring me to subscribe to -cvslog would be a reason to give up on trying to comply with the FFmpeg project's demands. The ratio of gratification to effort would be too discouraging. But, I'm just a newbie, and this policy decision is yours to make, not mine. >> Perhaps it can be reworded a bit to say it's encouraged for the >> cited reasons, but not mandatory if you just want to send a patch >> here and there? > If that really helps anybody, please do so! OK, I will modify the patch to include a new @Subheading describing subscription to ffmpeg-devel as in the current patch, and retaining a reworded version of the current @Subheading describing subscription to ffmpeg-cvslog as "encouraged for the cited reasons, but not mandatory if you just want to send a patch here and there" as Derek puts it. I won't get to that edit until the weekend. Derek, thank you for your intervention. Best regards, —Jim DeLaHunt, Vancouver, B.C.
diff --git a/doc/developer.texi b/doc/developer.texi index a7b4f1d737..9ac825ea5d 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -10,9 +10,7 @@ @contents -@chapter Developers Guide - -@section Notes for external developers +@chapter Notes for external developers This document is mostly useful for internal FFmpeg developers. External developers who need to use the API in their application should @@ -30,7 +28,7 @@ For more detailed legal information about the use of FFmpeg in external programs read the @file{LICENSE} file in the source tree and consult @url{https://ffmpeg.org/legal.html}. -@section Contributing +@chapter Contributing There are 3 ways by which code gets into FFmpeg. @itemize @bullet @@ -47,9 +45,9 @@ The developer making the commit and the author are responsible for their changes and should try to fix issues their commit causes. @anchor{Coding Rules} -@section Coding Rules +@chapter Coding Rules -@subsection Code formatting conventions +@section Code formatting conventions There are the following guidelines regarding the indentation in files: @@ -74,7 +72,7 @@ The presentation is one inspired by 'indent -i4 -kr -nut'. The main priority in FFmpeg is simplicity and small code size in order to minimize the bug count. -@subsection Comments +@section Comments Use the JavaDoc/Doxygen format (see examples below) so that code documentation can be generated automatically. All nontrivial functions should have a comment above them explaining what the function does, even if it is just one sentence. @@ -114,7 +112,7 @@ int myfunc(int my_parameter) ... @end example -@subsection C language features +@section C language features FFmpeg is programmed in the ISO C90 language with a few additional features from ISO C99, namely: @@ -160,7 +158,7 @@ mixing statements and declarations; GCC statement expressions (@samp{(x = (@{ int y = 4; y; @})}). @end itemize -@subsection Naming conventions +@section Naming conventions All names should be composed with underscores (_), not CamelCase. For example, @samp{avfilter_get_video_buffer} is an acceptable function name and @samp{AVFilterGetVideo} is not. The exception from this are type names, like @@ -204,7 +202,7 @@ letter as they are reserved by the C standard. Names starting with @code{_} are reserved at the file level and may not be used for externally visible symbols. If in doubt, just avoid names starting with @code{_} altogether. -@subsection Miscellaneous conventions +@section Miscellaneous conventions @itemize @bullet @item @@ -216,7 +214,7 @@ Casts should be used only when necessary. Unneeded parentheses should also be avoided if they don't make the code easier to understand. @end itemize -@subsection Editor configuration +@section Editor configuration In order to configure Vim to follow FFmpeg formatting conventions, paste the following snippet into your @file{.vimrc}: @example @@ -249,9 +247,9 @@ For Emacs, add these roughly equivalent lines to your @file{.emacs.d/init.el}: (setq c-default-style "ffmpeg") @end lisp -@section Development Policy +@chapter Development Policy -@subsection Patches/Committing +@section Patches/Committing @subheading 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}, @@ -350,7 +348,7 @@ 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! -@subsection Code +@section Code @subheading 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. @@ -381,12 +379,19 @@ 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. -@subsection Documentation/Other -@subheading 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. +@section Documentation/Other +@subheading Subscribe to the ffmpeg-devel mailing list. +It is important to be subscribed to the +@uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} +mailing list, because any patch you contribute must be sent there +and reviewed by the other developers. They may have comments about your +contribution. We expect you see those comments, and to improve your contribution +if requested. +Also, this list is where bugs and possible improvements or +general questions regarding commits are discussed. That may be helpful +information as you write your contribution. Finally, by being a list +subscriber your contribution will be posted immediately to the list, +without the moderation hold which messages from non-subscribers experience. @subheading Keep the documentation up to date. Update the documentation if you change behavior or add features. If you are @@ -406,7 +411,7 @@ finding a new maintainer and also don't forget to update the @file{MAINTAINERS} We think our rules are not too hard. If you have comments, contact us. -@section Code of conduct +@chapter Code of conduct Be friendly and respectful towards others and third parties. Treat others the way you yourself want to be treated. @@ -436,7 +441,7 @@ Finally, keep in mind the immortal words of Bill and Ted, "Be excellent to each other." @anchor{Submitting patches} -@section Submitting patches +@chapter Submitting patches First, read the @ref{Coding Rules} above if you did not yet, in particular the rules regarding patch submission. @@ -485,7 +490,7 @@ Give us a few days to react. But if some time passes without reaction, send a reminder by email. Your patch should eventually be dealt with. -@section New codecs or formats checklist +@chapter New codecs or formats checklist @enumerate @item @@ -537,7 +542,7 @@ Did you make sure it compiles standalone, i.e. with @end enumerate -@section patch submission checklist +@chapter patch submission checklist @enumerate @item @@ -650,7 +655,7 @@ Test your code with valgrind and or Address Sanitizer to ensure it's free of leaks, out of array accesses, etc. @end enumerate -@section Patch review process +@chapter Patch review process All patches posted to ffmpeg-devel will be reviewed, unless they contain a clear note that the patch is not for the git master branch. @@ -681,7 +686,7 @@ to be reviewed, please consider helping to review other patches, that is a great way to get everyone's patches reviewed sooner. @anchor{Regression tests} -@section Regression tests +@chapter Regression tests Before submitting a patch (or committing to the repository), you should at least test that you did not break anything. @@ -692,7 +697,7 @@ Running 'make fate' accomplishes this, please see @url{fate.html} for details. this case, the reference results of the regression tests shall be modified accordingly]. -@subsection Adding files to the fate-suite dataset +@section Adding files to the fate-suite dataset When there is no muxer or encoder available to generate test media for a specific test then the media has to be included in the fate-suite. @@ -703,7 +708,7 @@ Once you have a working fate test and fate sample, provide in the commit message or introductory message for the patch series that you post to the ffmpeg-devel mailing list, a direct link to download the sample media. -@subsection Visualizing Test Coverage +@section Visualizing Test Coverage The FFmpeg build system allows visualizing the test coverage in an easy manner with the coverage tools @code{gcov}/@code{lcov}. This involves @@ -730,7 +735,7 @@ You can use the command @code{make lcov-reset} to reset the coverage measurements. You will need to rerun @code{make lcov} after running a new test. -@subsection Using Valgrind +@section Using Valgrind The configure script provides a shortcut for using valgrind to spot bugs related to memory handling. Just add the option @@ -744,7 +749,7 @@ In case you need finer control over how valgrind is invoked, use the your configure line instead. @anchor{Release process} -@section Release process +@chapter Release process FFmpeg maintains a set of @strong{release branches}, which are the recommended deliverable for system integrators and distributors (such as @@ -776,7 +781,7 @@ adjustments to the symbol versioning file. Please discuss such changes on the @strong{ffmpeg-devel} mailing list in time to allow forward planning. @anchor{Criteria for Point Releases} -@subsection Criteria for Point Releases +@section Criteria for Point Releases Changes that match the following criteria are valid candidates for inclusion into a point release: @@ -800,7 +805,7 @@ point releases of the same release branch. The order for checking the rules is (1 OR 2 OR 3) AND 4. -@subsection Release Checklist +@section Release Checklist The release process involves the following steps:
Previously, the Developer Documentation <ffmpeg.org/developer.html> contained a single chapter, 1. Developer Guide, with all content under that single chapter. Thus the document structure was one level deeper and more complicated than it needed to be. It differed from similar documents such as /faq.html, which have multiple chapters. Also, the Developer Documentation had instructions to subscribe to the ffmpeg-cvslog email list. But that is no longer accurate. For the purposes in this section -- review of patches, discussion of development issues -- ffmpeg_devel is the appropriate email list. 1. Eliminate the single chapter, and promote each section underneath to chapter, and each subsection to section. Thus content and relative structure remains the same, but the overall structure is simpler. Anchors within the page remain the same. 2. Change ffmpeg-cvslog to ffmpeg-devel, and rewrite the wording of that section to match the current usage of ffmpeg-devel and to flow smoothly. I believe there were no links to the eliminated "Developer Documentation" chapter, based on a search of the source code. There are a lot of improvements possible to the Developer Documentation page, beyond this refactoring. However, making those improvements is a much bigger and more difficult task. This change is "low hanging fruit". Signed-off-by: Jim DeLaHunt <from.ffmpeg-dev@jdlh.com> --- doc/developer.texi | 69 +++++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 32 deletions(-)