Re: bug-introducing patches
From: Willy Tarreau
Date: Wed May 02 2018 - 16:02:46 EST
On Wed, May 02, 2018 at 07:42:33PM +0000, Sasha Levin wrote:
> I'm not advocating to keep bugs in. If there is a fix, but the developer
> can't indicate that proper testing was done on the fix we should revert
> the new feature rather than merge the untested fix in.
If you're exclusively talking about newly merged features, I agree. But
I think that the initial point was not just about newly merged features.
Sometimes it will not work because other changes rely on this new feature
but the way I see it is that this kind of back-pressure should work well
enough to encourage developers to show they have valid reasons to trust
their fix.
> The way I see it, if a commit can get one or two tested-by, it's a good
> alternative to a week in -next.
Agreed. Even their own actually. And I'm not kidding. Those who run large
amounts of tests on certain patches could really mention is in tested-by,
as opposed to the most common cases where the code was just regularly
tested.
> >After all, the person proposing a fix always knows better than anyone
> >else if this fix was done seriously or not. Developers who do lots of
> >testing before sending should not be penalized, and should get their
> >fix merged immediately. Those who just send untested patches should be
> >trusted much less.
>
> I'm a bit worried about (social) side effects of a scheme like this.
Me as well, because it's still a bit early for this, people might not
be prepared to this yet. But if it were at least discussed and presented
as one of the possibilities for the long term, newcomers would arrive
here with this possibility in mind and would possibly join in better
conditions and possibly that ultimately this solution would only exist
as a threat against bad players but would never be used. Also there are
more and more places where people find it normal to be noted by others,
maybe it will really end up like this over the long term, who knows. At
the very least a first note for a contractor is "I contributed X commits
last year, my work never got reverted for bad quality".
> >> From what I see, the same number of bugs-per-line-of-code applies for
> >> commits accross all -rc releases, so while it makes sense to get a fix
> >> in quickly at -rc1 to allow testing to continue, the same must not
> >> happen during -rc8, but unfourtenately it does now.
> >
> >That's where I strongly disagree, since it would mean releasing with even
> >more bugs than today.
>
> Just don't release it. If we don't have a tested fix for a reported
> regression either extend the release cycle (-rc10+) or just revert the
> new feature and get it in the next merge window.
I agree in general, but the reality will often be different (think about
contractors for a limited time as I suggested). It should be considered
as a penalty against improper testing so that we don't even have to reach
this situation.
Willy