| Message ID | 20170225145949.11516-1-nfxjfg@googlemail.com |
|---|---|
| State | New |
| Headers | show |
On 2/25/17, wm4 <nfxjfg@googlemail.com> wrote: > I'm documenting existing practice. > > I'm pulling the "6 months" timeout out of my ass, but I think it's > pretty suitable. > --- > doc/developer.texi | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/doc/developer.texi b/doc/developer.texi > index dbe1f5421f..41551498ad 100644 > --- a/doc/developer.texi > +++ b/doc/developer.texi > @@ -338,13 +338,21 @@ you applied the patch. > When applying patches that have been discussed (at length) on the mailing > list, reference the thread in the log message. > > -@subheading Always wait long enough before pushing changes > +@subheading Always wait long enough before pushing changes to code actively > maintained by others > Do NOT commit to code actively maintained by others without permission. > 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! > > +@subheading Pushing patches without sending them to the mailing list > +If you're the maintainer of a file, or the file is not actively maintained > by > +anyone, or the file is not covered by the MAINTAINERS file, you can just > push > +them without asking for permission, and without sending them to > ffmpeg-devel. > +This right only applies to developers with git push access, of course. > +A maintainer is considered not active if he hasn't posted a mail to > ffmpeg-devel > +for longer than 6 months, and hasn't pushed a patch in that time period > himself. > + > @subsection Code > @subheading API/ABI changes should be discussed before they are made. > Do not change behavior of the programs (renaming options etc) or public I object on this. I do prefer all the code to go though the maillist. Even trivial changes.
On Sat, 25 Feb 2017 17:03:58 +0200 Ivan Kalvachev <ikalvachev@gmail.com> wrote: > On 2/25/17, wm4 <nfxjfg@googlemail.com> wrote: > > I'm documenting existing practice. > > > > I'm pulling the "6 months" timeout out of my ass, but I think it's > > pretty suitable. > > --- > > doc/developer.texi | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/doc/developer.texi b/doc/developer.texi > > index dbe1f5421f..41551498ad 100644 > > --- a/doc/developer.texi > > +++ b/doc/developer.texi > > @@ -338,13 +338,21 @@ you applied the patch. > > When applying patches that have been discussed (at length) on the mailing > > list, reference the thread in the log message. > > > > -@subheading Always wait long enough before pushing changes > > +@subheading Always wait long enough before pushing changes to code actively > > maintained by others > > Do NOT commit to code actively maintained by others without permission. > > 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! > > > > +@subheading Pushing patches without sending them to the mailing list > > +If you're the maintainer of a file, or the file is not actively maintained > > by > > +anyone, or the file is not covered by the MAINTAINERS file, you can just > > push > > +them without asking for permission, and without sending them to > > ffmpeg-devel. > > +This right only applies to developers with git push access, of course. > > +A maintainer is considered not active if he hasn't posted a mail to > > ffmpeg-devel > > +for longer than 6 months, and hasn't pushed a patch in that time period > > himself. > > + > > @subsection Code > > @subheading API/ABI changes should be discussed before they are made. > > Do not change behavior of the programs (renaming options etc) or public > > I object on this. > > I do prefer all the code to go though the maillist. > Even trivial changes. That doesn't reflect existing practice. I'm only documenting existing practice. If you want to change the rules, post a separate patch.
On 25 February 2017 at 14:59, wm4 <nfxjfg@googlemail.com> wrote: > I'm documenting existing practice. > > I'm pulling the "6 months" timeout out of my ass, but I think it's > pretty suitable. > --- > doc/developer.texi | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/doc/developer.texi b/doc/developer.texi > index dbe1f5421f..41551498ad 100644 > --- a/doc/developer.texi > +++ b/doc/developer.texi > @@ -338,13 +338,21 @@ you applied the patch. > When applying patches that have been discussed (at length) on the mailing > list, reference the thread in the log message. > > -@subheading Always wait long enough before pushing changes > +@subheading Always wait long enough before pushing changes to code > actively maintained by others > Do NOT commit to code actively maintained by others without permission. > 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! > > +@subheading Pushing patches without sending them to the mailing list > +If you're the maintainer of a file, or the file is not actively > maintained by > +anyone, or the file is not covered by the MAINTAINERS file, you can just > push > +them without asking for permission, and without sending them to > ffmpeg-devel. > +This right only applies to developers with git push access, of course. > +A maintainer is considered not active if he hasn't posted a mail to > ffmpeg-devel > +for longer than 6 months, and hasn't pushed a patch in that time period > himself. > + > @subsection Code > @subheading API/ABI changes should be discussed before they are made. > Do not change behavior of the programs (renaming options etc) or public > -- > 2.11.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Patch LGTM
On Sat, 25 Feb 2017, wm4 wrote: > I'm documenting existing practice. > > I'm pulling the "6 months" timeout out of my ass, but I think it's > pretty suitable. > --- > doc/developer.texi | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/doc/developer.texi b/doc/developer.texi > index dbe1f5421f..41551498ad 100644 > --- a/doc/developer.texi > +++ b/doc/developer.texi > @@ -338,13 +338,21 @@ you applied the patch. > When applying patches that have been discussed (at length) on the mailing > list, reference the thread in the log message. > > -@subheading Always wait long enough before pushing changes > +@subheading Always wait long enough before pushing changes to code actively maintained by others > Do NOT commit to code actively maintained by others without permission. > 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! > > +@subheading Pushing patches without sending them to the mailing list > +If you're the maintainer of a file, or the file is not actively maintained by > +anyone, or the file is not covered by the MAINTAINERS file, you can just push > +them without asking for permission, and without sending them to ffmpeg-devel. > +This right only applies to developers with git push access, of course. > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel > +for longer than 6 months, and hasn't pushed a patch in that time period himself. > + Instead of this, I'd prefer all patches on the ML. Exceptions can be OKish (e.g. libav merges, which are hard as they are, or very trivial fixes), but I definitely would not make unreviewed pushes an encouraged and written policy. If this gets pushed, I am inclined to clean up the MAINTAINERS file and remove everybody who is no longer "active", and assume maintainership of parts I actively use and care about, but which has no maintainership anymore. Regards, Marton
On 25 February 2017 at 18:35, Marton Balint <cus@passwd.hu> wrote: > > On Sat, 25 Feb 2017, wm4 wrote: > > I'm documenting existing practice. >> >> I'm pulling the "6 months" timeout out of my ass, but I think it's >> pretty suitable. >> --- >> doc/developer.texi | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/doc/developer.texi b/doc/developer.texi >> index dbe1f5421f..41551498ad 100644 >> --- a/doc/developer.texi >> +++ b/doc/developer.texi >> @@ -338,13 +338,21 @@ you applied the patch. >> When applying patches that have been discussed (at length) on the mailing >> list, reference the thread in the log message. >> >> -@subheading Always wait long enough before pushing changes >> +@subheading Always wait long enough before pushing changes to code >> actively maintained by others >> Do NOT commit to code actively maintained by others without permission. >> 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! >> >> +@subheading Pushing patches without sending them to the mailing list >> +If you're the maintainer of a file, or the file is not actively >> maintained by >> +anyone, or the file is not covered by the MAINTAINERS file, you can just >> push >> +them without asking for permission, and without sending them to >> ffmpeg-devel. >> +This right only applies to developers with git push access, of course. >> +A maintainer is considered not active if he hasn't posted a mail to >> ffmpeg-devel >> +for longer than 6 months, and hasn't pushed a patch in that time period >> himself. >> + >> > > Instead of this, I'd prefer all patches on the ML. Exceptions can be OKish > (e.g. libav merges, which are hard as they are, or very trivial fixes), but > I definitely would not make unreviewed pushes an encouraged and written > policy. > > This is the way it has been done for years and its the way the project has been able to move as rapidly as it has. That would slow down anything large from being developed in the codebase, like encoders or decoders for a new format, which are usually being developed by a single person who understands the code better than anybody. I am okay with it being an unwritten rule, anyone who needs to know about it knows about it and everyone that knows about it knows that moderation is the key. But forbidding it will kill the project. > If this gets pushed, I am inclined to clean up the MAINTAINERS file and > remove everybody who is no longer "active", and assume maintainership of > parts I actively use and care about, but which has no maintainership > anymore. > So you're okay as long as maintainers gets sorted out? You might as well do it, its what's the file is supposed to reflect.
On Sat, 25 Feb 2017, Rostislav Pehlivanov wrote: > On 25 February 2017 at 18:35, Marton Balint <cus@passwd.hu> wrote: > >> >> On Sat, 25 Feb 2017, wm4 wrote: >> >> I'm documenting existing practice. >>> >>> I'm pulling the "6 months" timeout out of my ass, but I think it's >>> pretty suitable. >>> --- [...] > This is the way it has been done for years and its the way the project has > been able to move as rapidly as it has. That would slow down anything large > from being developed in the codebase, like encoders or decoders for a new > format, which are usually being developed by a single person who > understands the code better than anybody. I am okay with it being an > unwritten rule, anyone who needs to know about it knows about it and > everyone that knows about it knows that moderation is the key. But > forbidding it will kill the project. I only want to have a chance to review before patches got pushed. I am not saying we should explicitly demand a review for each patch. So this normally should only cause any developer an additional sent email to the ML and 1-2 days of delay. With git, I don't think this is that much additional work. Of course, I could just subscribe to csvlog as well, and give post-commit reviews if I want, it is just better to do it earlier, and less chance of revert wars, because with the 'written' rule above I just as easily revert anything in unmaintained code without a discussion, and remain within the 'rules'. >> If this gets pushed, I am inclined to clean up the MAINTAINERS file and >> remove everybody who is no longer "active", and assume maintainership of >> parts I actively use and care about, but which has no maintainership >> anymore. >> > > So you're okay as long as maintainers gets sorted out? You might as well do > it, its what's the file is supposed to reflect. I still prefer if this is not applied. MAINTAINERS file might be cleaned up regardless of this patch, however I think we should at least ping everybody we are trying to remove from it. Regards, Marton
On Sat, Feb 25, 2017, at 11:23 AM, Marton Balint wrote: > > I only want to have a chance to review before patches got pushed. I am > not > saying we should explicitly demand a review for each patch. So this > normally should only cause any developer an additional sent email to the > ML and 1-2 days of delay. With git, I don't think this is that much > additional work. One downside to a forced delay is that patches will occasionally be completely forgotten instead of being applied. Patchwork helps, but not everyone uses it or maintains the status of their patches.
On 25 February 2017 at 20:23, Marton Balint <cus@passwd.hu> wrote: > > On Sat, 25 Feb 2017, Rostislav Pehlivanov wrote: > > On 25 February 2017 at 18:35, Marton Balint <cus@passwd.hu> wrote: >> >> >>> On Sat, 25 Feb 2017, wm4 wrote: >>> >>> I'm documenting existing practice. >>> >>>> >>>> I'm pulling the "6 months" timeout out of my ass, but I think it's >>>> pretty suitable. >>>> --- >>>> >>> > [...] > > This is the way it has been done for years and its the way the project has >> been able to move as rapidly as it has. That would slow down anything >> large >> from being developed in the codebase, like encoders or decoders for a new >> format, which are usually being developed by a single person who >> understands the code better than anybody. I am okay with it being an >> unwritten rule, anyone who needs to know about it knows about it and >> everyone that knows about it knows that moderation is the key. But >> forbidding it will kill the project. >> > > I only want to have a chance to review before patches got pushed. I am not > saying we should explicitly demand a review for each patch. So this > normally should only cause any developer an additional sent email to the ML > and 1-2 days of delay. With git, I don't think this is that much additional > work. > > Review or not, too slow still. If I have a fix for something I wrote and understand I'm not willing to wait and I'll push it, if I have some doubts I'll post it to the ML. You seem to give people way less trust than they deserve, especially if they have commit rights. I can't accept that.
On Sat, Feb 25, 2017 at 11:47 PM, Rostislav Pehlivanov <atomnuker@gmail.com> wrote: >> >> This is the way it has been done for years and its the way the project has >>> been able to move as rapidly as it has. That would slow down anything >>> large >>> from being developed in the codebase, like encoders or decoders for a new >>> format, which are usually being developed by a single person who >>> understands the code better than anybody. I am okay with it being an >>> unwritten rule, anyone who needs to know about it knows about it and >>> everyone that knows about it knows that moderation is the key. But >>> forbidding it will kill the project. >>> >> >> I only want to have a chance to review before patches got pushed. I am not >> saying we should explicitly demand a review for each patch. So this >> normally should only cause any developer an additional sent email to the ML >> and 1-2 days of delay. With git, I don't think this is that much additional >> work. >> >> > Review or not, too slow still. If I have a fix for something I wrote and > understand I'm not willing to wait and I'll push it, if I have some doubts > I'll post it to the ML. You seem to give people way less trust than they > deserve, especially if they have commit rights. I can't accept that. I would say that if you are the maintainer of something, you should be able to push to it, but still be encouraged to post to the ML with bigger changes (not simple fixes), but things that are unmaintained or maintained by someone else should generally go through the ML. Thats how most had interpreted the policy in the past, I would think, even if it didn't agree with the written policy entirely. - Hendrik
On Sat, 25 Feb 2017 21:23:27 +0100 (CET) Marton Balint <cus@passwd.hu> wrote: > On Sat, 25 Feb 2017, Rostislav Pehlivanov wrote: > > > On 25 February 2017 at 18:35, Marton Balint <cus@passwd.hu> wrote: > > > >> > >> On Sat, 25 Feb 2017, wm4 wrote: > >> > >> I'm documenting existing practice. > >>> > >>> I'm pulling the "6 months" timeout out of my ass, but I think it's > >>> pretty suitable. > >>> --- > > [...] > > > This is the way it has been done for years and its the way the project has > > been able to move as rapidly as it has. That would slow down anything large > > from being developed in the codebase, like encoders or decoders for a new > > format, which are usually being developed by a single person who > > understands the code better than anybody. I am okay with it being an > > unwritten rule, anyone who needs to know about it knows about it and > > everyone that knows about it knows that moderation is the key. But > > forbidding it will kill the project. > > I only want to have a chance to review before patches got pushed. I am not > saying we should explicitly demand a review for each patch. So this > normally should only cause any developer an additional sent email to the > ML and 1-2 days of delay. With git, I don't think this is that much > additional work. > > Of course, I could just subscribe to csvlog as well, and give post-commit > reviews if I want, it is just better to do it earlier, and less chance of > revert wars, because with the 'written' rule above I just as easily revert > anything in unmaintained code without a discussion, and remain within the > 'rules'. > > > >> If this gets pushed, I am inclined to clean up the MAINTAINERS file and > >> remove everybody who is no longer "active", and assume maintainership of > >> parts I actively use and care about, but which has no maintainership > >> anymore. > >> > > > > So you're okay as long as maintainers gets sorted out? You might as well do > > it, its what's the file is supposed to reflect. > > I still prefer if this is not applied. MAINTAINERS file might be cleaned > up regardless of this patch, however I think we should at least ping > everybody we are trying to remove from it. As I said in another reply, I'm only documenting existing practice. We can still change the rules in a separate patch (which of course will be major drama etc. in which I don't necessarily want to be involved). Or are there any doubts that this patch does what I claim it does (documenting existing practice)? Apart from that, I don't even know whether I can push this patch, or what I have to do to get clear rejection/approval. The MAINTAINERS file has ironically no maintainer. So far, the score is 1 approval, and 2 invalid rejects. I guess I'll push it on Friday morning if nobody rejects it by then.
2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > I'm documenting existing practice. > -@subheading Always wait long enough before pushing changes > +@subheading Always wait long enough before pushing changes to code actively maintained by others I suspect this is missing an exception for security issues if you want to document the actual practice. > +@subheading Pushing patches without sending them to the mailing list > +If you're the maintainer of a file, or the file is not actively maintained by > +anyone, or the file is not covered by the MAINTAINERS file, you can just push > +them without asking for permission, and without sending them to ffmpeg-devel. > +This right only applies to developers with git push access, of course. > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel > +for longer than 6 months, and hasn't pushed a patch in that time period himself. Unfortunately, there are maintainers who are happy to review patches sent to improve their code but the files were not touched for more than six months so they did not seem active for more than six months. Carl Eugen
2017-02-25 16:03 GMT+01:00 Ivan Kalvachev <ikalvachev@gmail.com>: > I do prefer all the code to go though the maillist. > Even trivial changes. This is not how we have done it for more than a decade and I am against changing our (extremely successful) practice. Carl Eugen
2017-02-25 19:35 GMT+01:00 Marton Balint <cus@passwd.hu>: > Instead of this, I'd prefer all patches on the ML. This would mean we changed from a policy that we successful applied for over a decade to a policy of which we - to the best of my knowledge - know that it does not work well. > Exceptions can be OKish > (e.g. libav merges, which are hard as they are, or very trivial fixes), but > I definitely would not make unreviewed pushes an encouraged and written > policy. I was under the impression that this - luckily - already was written policy. > If this gets pushed, I am inclined to clean up the MAINTAINERS file and > remove everybody who is no longer "active" While a cleanup is of course ok, please note that there MAINTAINERS who are willing to maintain but had no reason to do so for longer than six months. Carl Eugen
2017-02-25 21:23 GMT+01:00 Marton Balint <cus@passwd.hu>:
> Of course, I could just subscribe to csvlog as well
I was under the impression that if you want commit rights,
you are required to be subscribed to -cvslog.
Carl Eugen
On Wed, 1 Mar 2017 12:20:10 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > I'm documenting existing practice. > > > -@subheading Always wait long enough before pushing changes > > +@subheading Always wait long enough before pushing changes to code actively maintained by others > > I suspect this is missing an exception for security issues if you want to > document the actual practice. I can add to the end of the subheading: Critical security issues are an exception. These can be pushed after the patch has been for 24 hours on the mailing list and the maintainer didn't respond yet, and nobody has rejected the patch. In addition, if another committer has approved the patch and acknowledged the urgency of the fix, the patch can be pushed immediately. Maybe a bit long, but should cover all bases. > > +@subheading Pushing patches without sending them to the mailing list > > +If you're the maintainer of a file, or the file is not actively maintained by > > +anyone, or the file is not covered by the MAINTAINERS file, you can just push > > +them without asking for permission, and without sending them to ffmpeg-devel. > > +This right only applies to developers with git push access, of course. > > > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel > > +for longer than 6 months, and hasn't pushed a patch in that time period himself. > > Unfortunately, there are maintainers who are happy to review patches > sent to improve their code but the files were not touched for more than > six months so they did not seem active for more than six months. So what is a reasonable method of determining whether a maintainer is reachable? The worst part is that not even "active" maintainers always respond, even if you go a timeout.
2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Wed, 1 Mar 2017 12:20:10 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > I'm documenting existing practice. >> >> > -@subheading Always wait long enough before pushing changes >> > +@subheading Always wait long enough before pushing changes >> > to code actively maintained by others >> >> I suspect this is missing an exception for security issues if you want to >> document the actual practice. > > I can add to the end of the subheading: > > Critical security issues are an exception. These can be pushed after > the patch has been for 24 hours on the mailing list and the maintainer > didn't respond yet, and nobody has rejected the patch. In addition, > if another committer has approved the patch and acknowledged the > urgency of the fix, the patch can be pushed immediately. I will most likely not fix a (real) security issue, but above seems quite unpractical to me (and unreasonable for real security issues). > Maybe a bit long, but should cover all bases. > >> > +@subheading Pushing patches without sending them to the mailing list >> > +If you're the maintainer of a file, or the file is not actively maintained by >> > +anyone, or the file is not covered by the MAINTAINERS file, you can just push >> > +them without asking for permission, and without sending them to ffmpeg-devel. >> > +This right only applies to developers with git push access, of course. >> >> > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel >> > +for longer than 6 months, and hasn't pushed a patch in that time period himself. >> >> Unfortunately, there are maintainers who are happy to review patches >> sent to improve their code but the files were not touched for more than >> six months so they did not seem active for more than six months. > > So what is a reasonable method of determining whether a maintainer > is reachable? I fear there is no strict definition, patches can always be reverted though if a maintainer requests that. I am just (slightly) against writing "after six months, you are no more a maintainer". > The worst part is that not even "active" maintainers always respond, > even if you go a timeout. Then you push after the timeout (if no delay was requested). Carl Eugen
On Wed, 1 Mar 2017 13:06:29 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Wed, 1 Mar 2017 12:20:10 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > I'm documenting existing practice. > >> > >> > -@subheading Always wait long enough before pushing changes > >> > +@subheading Always wait long enough before pushing changes > >> > to code actively maintained by others > >> > >> I suspect this is missing an exception for security issues if you want to > >> document the actual practice. > > > > I can add to the end of the subheading: > > > > Critical security issues are an exception. These can be pushed after > > the patch has been for 24 hours on the mailing list and the maintainer > > didn't respond yet, and nobody has rejected the patch. In addition, > > if another committer has approved the patch and acknowledged the > > urgency of the fix, the patch can be pushed immediately. > > I will most likely not fix a (real) security issue, but above seems quite > unpractical to me (and unreasonable for real security issues). How is that impractical? What do you suggest? I certainly hope you're not suggesting that security-sensitive changes to code the patch author does not maintain can be pushed without reviews at all. > > Maybe a bit long, but should cover all bases. > > > >> > +@subheading Pushing patches without sending them to the mailing list > >> > +If you're the maintainer of a file, or the file is not actively maintained by > >> > +anyone, or the file is not covered by the MAINTAINERS file, you can just push > >> > +them without asking for permission, and without sending them to ffmpeg-devel. > >> > +This right only applies to developers with git push access, of course. > >> > >> > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel > >> > +for longer than 6 months, and hasn't pushed a patch in that time period himself. > >> > >> Unfortunately, there are maintainers who are happy to review patches > >> sent to improve their code but the files were not touched for more than > >> six months so they did not seem active for more than six months. > > > > So what is a reasonable method of determining whether a maintainer > > is reachable? > > I fear there is no strict definition, patches can always be reverted though > if a maintainer requests that. > > I am just (slightly) against writing "after six months, you are no more a > maintainer". That's not what I wrote. It merely means that if you have showed no sign of activity after 6 months, the timeout can be skipped. > > The worst part is that not even "active" maintainers always respond, > > even if you go a timeout. > > Then you push after the timeout (if no delay was requested). michaelni didn't wait when pushing his vp56.c patches (didn't even send them to the mailing list), even though it was a mid-sized (i.e. not small) change. The maintainer of vp56.c is still reachable by mail, but hasn't posted or contributed anything to ffmpeg in 3.5 years. michaelni didn't wait for the timeout, which does not match with what you seem to demand. Please explain what the rules of this project are.
On Wed, Mar 01, 2017 at 01:31:02PM +0100, wm4 wrote: > On Wed, 1 Mar 2017 13:06:29 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > 2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > > On Wed, 1 Mar 2017 12:20:10 +0100 > > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > >> > I'm documenting existing practice. > > >> > > >> > -@subheading Always wait long enough before pushing changes > > >> > +@subheading Always wait long enough before pushing changes > > >> > to code actively maintained by others > > >> > > >> I suspect this is missing an exception for security issues if you want to > > >> document the actual practice. > > > > > > I can add to the end of the subheading: > > > > > > Critical security issues are an exception. These can be pushed after > > > the patch has been for 24 hours on the mailing list and the maintainer > > > didn't respond yet, and nobody has rejected the patch. In addition, > > > if another committer has approved the patch and acknowledged the > > > urgency of the fix, the patch can be pushed immediately. > > > > I will most likely not fix a (real) security issue, but above seems quite > > unpractical to me (and unreasonable for real security issues). > > How is that impractical? What do you suggest? > > I certainly hope you're not suggesting that security-sensitive changes > to code the patch author does not maintain can be pushed without reviews > at all. I dont know what carl meant exactly but as one doing many of the fixes there are a few words i should say Posting patches is only usefull if more issues are found in the posted patch as a result of it being posted than if it wasnt posted. It seems few issues are found in patches in this category when posted. Posting patches interrupts the development cycle, normally i would debug an issue, write a fix, test and review the fix and if iam unsure and theres someone who knows the code better post a patch (either privatly or publically as the case demands) otherwise push and backport and then the next issue. The cases where no patch is posted, the pushing and local backport happens immediately, but if i post a patch and wait 24h by the time i backport it to the releases its been a longer time and there have been many other issues in the meantime, that makes backports harder, less common and lower quality. Thats what iam seeing in the last days not a imagined issue. Also i think iam more sloppy with self reviewing my code when i post a patch Also adding a unconditional 24h delay for changes makes the whole process more work. The time i can spend on FFmpeg is limited, if i send a patch in every case and not just in cases were theres an active maitainer who wants and does review things before being pushed. Then the result is some other things arent done anymore simply due to lack of time, If you want a concrete example you can look at my gnu/kfreebsd fate client, it stoped working days ago and i noticed but simply didnt had the time to look into it, previously i always looked into such things quickly when i noticed. And posting about (serious) security issues in public before fixing them adds time for anyone wanting to exploit it posting patches and waiting has a cost and we should only pay that when we get more value back. On a slightly differnt topic, accusing other fellow developers also has a cost, not just everyones time but also at least for me, the will and interrest to work in this environment and now ill try to stay out of this (rather timeconsuming disscussion) like i did before, just wanted to send one mail as the discussion was about security and related patches which i work on alot ... thx [...]
On Thu, 2 Mar 2017 12:44:57 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 01, 2017 at 01:31:02PM +0100, wm4 wrote: > > On Wed, 1 Mar 2017 13:06:29 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > 2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > > > On Wed, 1 Mar 2017 12:20:10 +0100 > > > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > > > >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > > >> > I'm documenting existing practice. > > > >> > > > >> > -@subheading Always wait long enough before pushing changes > > > >> > +@subheading Always wait long enough before pushing changes > > > >> > to code actively maintained by others > > > >> > > > >> I suspect this is missing an exception for security issues if you want to > > > >> document the actual practice. > > > > > > > > I can add to the end of the subheading: > > > > > > > > Critical security issues are an exception. These can be pushed after > > > > the patch has been for 24 hours on the mailing list and the maintainer > > > > didn't respond yet, and nobody has rejected the patch. In addition, > > > > if another committer has approved the patch and acknowledged the > > > > urgency of the fix, the patch can be pushed immediately. > > > > > > I will most likely not fix a (real) security issue, but above seems quite > > > unpractical to me (and unreasonable for real security issues). > > > > How is that impractical? What do you suggest? > > > > I certainly hope you're not suggesting that security-sensitive changes > > to code the patch author does not maintain can be pushed without reviews > > at all. > > I dont know what carl meant exactly but as one doing many of the fixes > there are a few words i should say > > Posting patches is only usefull if more issues are found in the > posted patch as a result of it being posted than if it wasnt posted. > It seems few issues are found in patches in this category when posted. > Posting patches interrupts the development cycle, normally i would > debug an issue, write a fix, test and review the fix and if iam > unsure and theres someone who knows the code better post a patch > (either privatly or publically as the case demands) > otherwise push and backport and then the next issue. > The cases where no patch is posted, the pushing and local backport > happens immediately, but if i post a patch and wait 24h by the time i > backport it to the releases its been a longer time and there have been > many other issues in the meantime, that makes backports harder, > less common and lower quality. Thats what iam seeing in the last > days not a imagined issue. > Also i think iam more sloppy with self reviewing my code when i post > a patch > > Also adding a unconditional 24h delay for changes makes the whole > process more work. The time i can spend on FFmpeg is limited, if i > send a patch in every case and not just in cases were theres an > active maitainer who wants and does review things before being > pushed. Then the result is some other things arent done anymore simply > due to lack of time, If you want a concrete example you can look at > my gnu/kfreebsd fate client, it stoped working days ago and i noticed > but simply didnt had the time to look into it, previously i always > looked into such things quickly when i noticed. > > > And posting about (serious) security issues in public before fixing > them adds time for anyone wanting to exploit it > > posting patches and waiting has a cost and we should only pay that > when we get more value back. > > On a slightly differnt topic, accusing other fellow developers also > has a cost, not just everyones time but also at least for me, the > will and interrest to work in this environment > > and now ill try to stay out of this (rather timeconsuming disscussion) > like i did before, just wanted to send one mail as the discussion was > about security and related patches which i work on alot ... > > thx > > [...] It seems you didn't read what I wrote. If another committer says ok to the patch (even on IRC), it would be ok.
2017-03-01 13:31 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > On Wed, 1 Mar 2017 13:06:29 +0100 > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> > On Wed, 1 Mar 2017 12:20:10 +0100 >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> > >> >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: >> >> > I'm documenting existing practice. >> >> >> >> > -@subheading Always wait long enough before pushing changes >> >> > +@subheading Always wait long enough before pushing changes >> >> > to code actively maintained by others >> >> >> >> I suspect this is missing an exception for security issues if you want to >> >> document the actual practice. >> > >> > I can add to the end of the subheading: >> > >> > Critical security issues are an exception. These can be pushed after >> > the patch has been for 24 hours on the mailing list and the maintainer >> > didn't respond yet, and nobody has rejected the patch. In addition, >> > if another committer has approved the patch and acknowledged the >> > urgency of the fix, the patch can be pushed immediately. >> >> I will most likely not fix a (real) security issue, but above seems quite >> unpractical to me (and unreasonable for real security issues). > > How is that impractical? What do you suggest? Posting security issues and wait for comments does not seem like a useful approach. > I certainly hope you're not suggesting that security-sensitive changes > to code the patch author does not maintain can be pushed without reviews > at all. I believe this is the established practice that you want to document iiuc. >> > Maybe a bit long, but should cover all bases. >> > >> >> > +@subheading Pushing patches without sending them to the mailing list >> >> > +If you're the maintainer of a file, or the file is not actively maintained by >> >> > +anyone, or the file is not covered by the MAINTAINERS file, you can just push >> >> > +them without asking for permission, and without sending them to ffmpeg-devel. >> >> > +This right only applies to developers with git push access, of course. >> >> >> >> > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel >> >> > +for longer than 6 months, and hasn't pushed a patch in that time period himself. >> >> >> >> Unfortunately, there are maintainers who are happy to review patches >> >> sent to improve their code but the files were not touched for more than >> >> six months so they did not seem active for more than six months. >> > >> > So what is a reasonable method of determining whether a maintainer >> > is reachable? >> >> I fear there is no strict definition, patches can always be reverted though >> if a maintainer requests that. >> >> I am just (slightly) against writing "after six months, you are no more a >> maintainer". > > That's not what I wrote. But this is how the change of text can be interpreted. > It merely means that if you have showed no sign of activity after 6 > months, the timeout can be skipped. As said, this is not a sufficient sign for non-maintenance. >> > The worst part is that not even "active" maintainers always respond, >> > even if you go a timeout. >> >> Then you push after the timeout (if no delay was requested). > > michaelni didn't wait when pushing his vp56.c patches (didn't even send > them to the mailing list), even though it was a mid-sized (i.e. not > small) change. The patch did not change API (which would require the patch sent to the mailing list) and vp56.c is not actively maintained, so I don't understand what you are trying to say. > The maintainer of vp56.c is still reachable by mail, but > hasn't posted or contributed anything to ffmpeg in 3.5 years. michaelni > didn't wait for the timeout, which does not match with what you seem to > demand. No, you misunderstand: I am just pointing out that a pure time-out is not sufficient to decide on maintenance. > Please explain what the rules of this project are. As you know, I disagree with several of the rules, and I have said so. You have - afaict - agreed to them and I still feel that you prefer not to obey. I have no idea what I can explain to you. Generally, I honestly don't understand what you are trying to achieve with your emails. Carl Eugen
On Thu, 2 Mar 2017 17:34:11 +0100 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-03-01 13:31 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > > On Wed, 1 Mar 2017 13:06:29 +0100 > > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > >> 2017-03-01 12:36 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> > On Wed, 1 Mar 2017 12:20:10 +0100 > >> > Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> > > >> >> 2017-02-25 15:59 GMT+01:00 wm4 <nfxjfg@googlemail.com>: > >> >> > I'm documenting existing practice. > >> >> > >> >> > -@subheading Always wait long enough before pushing changes > >> >> > +@subheading Always wait long enough before pushing changes > >> >> > to code actively maintained by others > >> >> > >> >> I suspect this is missing an exception for security issues if you want to > >> >> document the actual practice. > >> > > >> > I can add to the end of the subheading: > >> > > >> > Critical security issues are an exception. These can be pushed after > >> > the patch has been for 24 hours on the mailing list and the maintainer > >> > didn't respond yet, and nobody has rejected the patch. In addition, > >> > if another committer has approved the patch and acknowledged the > >> > urgency of the fix, the patch can be pushed immediately. > >> > >> I will most likely not fix a (real) security issue, but above seems quite > >> unpractical to me (and unreasonable for real security issues). > > > > How is that impractical? What do you suggest? > > Posting security issues and wait for comments does not seem > like a useful approach. > > > I certainly hope you're not suggesting that security-sensitive changes > > to code the patch author does not maintain can be pushed without reviews > > at all. > > I believe this is the established practice that you want to > document iiuc. > > >> > Maybe a bit long, but should cover all bases. > >> > > >> >> > +@subheading Pushing patches without sending them to the mailing list > >> >> > +If you're the maintainer of a file, or the file is not actively maintained by > >> >> > +anyone, or the file is not covered by the MAINTAINERS file, you can just push > >> >> > +them without asking for permission, and without sending them to ffmpeg-devel. > >> >> > +This right only applies to developers with git push access, of course. > >> >> > >> >> > +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel > >> >> > +for longer than 6 months, and hasn't pushed a patch in that time period himself. > >> >> > >> >> Unfortunately, there are maintainers who are happy to review patches > >> >> sent to improve their code but the files were not touched for more than > >> >> six months so they did not seem active for more than six months. > >> > > >> > So what is a reasonable method of determining whether a maintainer > >> > is reachable? > >> > >> I fear there is no strict definition, patches can always be reverted though > >> if a maintainer requests that. > >> > >> I am just (slightly) against writing "after six months, you are no more a > >> maintainer". > > > > That's not what I wrote. > > But this is how the change of text can be interpreted. > > > It merely means that if you have showed no sign of activity after 6 > > months, the timeout can be skipped. > > As said, this is not a sufficient sign for non-maintenance. > > >> > The worst part is that not even "active" maintainers always respond, > >> > even if you go a timeout. > >> > >> Then you push after the timeout (if no delay was requested). > > > > michaelni didn't wait when pushing his vp56.c patches (didn't even send > > them to the mailing list), even though it was a mid-sized (i.e. not > > small) change. > > The patch did not change API (which would require the patch > sent to the mailing list) and vp56.c is not actively maintained, so > I don't understand what you are trying to say. > > > The maintainer of vp56.c is still reachable by mail, but > > hasn't posted or contributed anything to ffmpeg in 3.5 years. michaelni > > didn't wait for the timeout, which does not match with what you seem to > > demand. > > No, you misunderstand: > I am just pointing out that a pure time-out is not sufficient to > decide on maintenance. > > > Please explain what the rules of this project are. > > As you know, I disagree with several of the rules, and I have said so. > You have - afaict - agreed to them and I still feel that you prefer not > to obey. I have no idea what I can explain to you. > > Generally, I honestly don't understand what you are trying to > achieve with your emails. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel I give up. Please write a patch to clarify the rules yourself. Otherwise I'm going to assume that the rules are as fuzzy as they seem to be, and I'll adjust my behavior accordingly.
diff --git a/doc/developer.texi b/doc/developer.texi index dbe1f5421f..41551498ad 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -338,13 +338,21 @@ you applied the patch. When applying patches that have been discussed (at length) on the mailing list, reference the thread in the log message. -@subheading Always wait long enough before pushing changes +@subheading Always wait long enough before pushing changes to code actively maintained by others Do NOT commit to code actively maintained by others without permission. 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! +@subheading Pushing patches without sending them to the mailing list +If you're the maintainer of a file, or the file is not actively maintained by +anyone, or the file is not covered by the MAINTAINERS file, you can just push +them without asking for permission, and without sending them to ffmpeg-devel. +This right only applies to developers with git push access, of course. +A maintainer is considered not active if he hasn't posted a mail to ffmpeg-devel +for longer than 6 months, and hasn't pushed a patch in that time period himself. + @subsection Code @subheading API/ABI changes should be discussed before they are made. Do not change behavior of the programs (renaming options etc) or public