Re: Automating basic patch tests...

Riley Williams (rhw@bigfoot.com)
Thu, 1 Oct 1998 14:01:27 +0100 (GMT)


Hi Helge.

On Thu, 1 Oct 1998, Helge Hafting wrote:

>> As a very basic minimum, I'd like to suggest that it perform the
>> following tests:

> [good tests snipped]

>> 3. How many different files does the patch modify? Should there
>> be a limit on this number, to discourage submission of patches
>> that change too much at once?

>> 4. (As an alternative to 3) How many different directories does
>> the patch modify files in?

> I believe there will be infrequent occations where a patch
> has to modify files "all over the place". If those are auto-
> rejected then some alternate mechanism must be used for such
> occations. (Such as emailing Linus directly)

I agree there, but would suggest that a patch that failed either of
the above tests was in need of serious scrutiny BEFORE any sort of
consideration is given as to its compilability - see below for how I
see the patch submittal process going...

> Not good, because people getting rejects for other reasons may
> decide to use the alternate approach too.

Agreed.

> Instead of auto-rejecting just send the author a warning that he
> ought to be careful with this type of patch, and mark it as
> "dubious". Only real people can make a good decision.

Like I said, the purpose of those two tests isn't to reject the
patches, but to determine whether they are good candidates for
auto-verification or should be passed directly to a human verifier.

> If such patches become a workload problem anyway, forward them to
> someone else who can see if they make sense before possibly passing
> them on to Linus.

I would see the patch submittal procedure as going something like the
following sketch diagram...

---------------------------
[ Patch submitted by author ]
---------------------------
|
------------------------------
[ Auto-verifier checks patches ]
------------------------------
| | |
Query Reject Accept
| | |
| | ----------------
| | [ Auto-compiler ]
| | [ checks patches ]
| | ----------------
| | | |
| | Reject Accept
| | | |
| |-<------+ |
| | |
| ------------------ |
| [ Email rejection ] |
| [ to patch author ] |
| [ with explanation ] |
| ------------------ |
| | |
| --------------- |
| [ Discard patch ] |
| --------------- |
| |
+------------>-+-<-------------+
|
-------------------------
[ Forward patch to most ]
[ relevant human verifier ]
-------------------------

How does that look to you?

I would also add that the explanation should include suggestions as to
measures the author could take to make the patch more acceptable to
the system. Items in this list could include...

1. If the patch fails to apply cleanly against the current kernel,
suggest that the author upgrade to the current sources, then
apply the patch to them and submit the result.

2. If the patch fails to compile against the current kernel, supply
the compiler's error messages and ask the author to investigate
and submit a corrected patch.

3. Some patches will duplicate the results of other patches. If this
is detected, it might be an idea to suggest that the patch authors
concerned contact each other to discuss their ideas.

I'm sure other suggestions will soon come to light as well...

Best wishes from Riley.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/