[OpenAFS-devel] warnings fix

[Russ Allbery]

Marcus Watts <mdw@umich.edu> writes:

> Then we really do have a problem.
>
> The current process is adapted to small changes.  It deals with medium
> changes inefficiently.  It fails to address big changes at all.

Yes.  Big changes have to become smaller, or it's going to be increasingly
difficult to get them incorporated.  Large, monolithic patches are going
to be incredibly hard to review and accept.  As we've mentioned, we're
going to try hard to help with the large chunks of code that are already
outstanding, but I want to keep beating on this drum for any future
development from this point on.

This problem is not unique to AFS.  This is the case for approximately
every large open source software project I've ever seen.  Those that do
take large code landings develop them on branches with code review of
every patch going into that branch, so even when the large landing on
trunk is still a giant change, everything goes through the same code
review process as separate smaller changes.

It's quite possible that there will be other people in the community who
would be willing to help you take patches apart and turn them into smaller
conceptual changes that can be reviewed.  (In fact, I'm possibly one of
them, although not this month.)

> I made an early foray with cvs head and rxk5.  I gave up because my
> builds broke so often I spent all my time working on those bugs rather
> than on rxk5.  The above change history that Russ detailed shows this
> problem exists today just as much with gerrit; so the problem here is
> not with the technology.

If there are structural changes that you need to ensure that your code
remains buildable, those are the ones that need to get into the tree
first.

It's a fundamental mindset shift in how development is done.  Large
projects can't take large code drops in one lump.  The Linux kernel
doesn't, gcc doesn't, and we can't.  The logistics are just impossible.
The corresponding obligation on the Gatekeepers is to try to review
patches quickly so as not to stall long chains of development.  Git
provides additional tools to help and is far, far better at merging than
CVS.

I want to help as much as possible with the work that was done before the
shift to Git and to Gerrit, since I really understand how daunting it can
be to feed small changes in a CVS world.  But going forward, for new
development from now on, one simply cannot go off for a while, work on a
significant project that touches lots of code, and then come back with a
completed patch and expect it to be integrated.  That's always going to be
a nightmare.  It has to come in managable pieces somehow or another.
Breaking it up after the fact is usually way harder than developing it in
chunks to start with.

> With most of my recent rxk5 work, I stuck to 1.5.x releases as "stable
> points".  This was helpful except when the volume code broke.  :-(

One of the things that we're requiring with Gerrit is verification of each
patch before it goes into the tree, meaning that it will at least be
guaranteed to compile somewhere.  For complex patches, we want to do more
testing.  Obviously, our test suite isn't where we need it to be to do a
lot of automated verification, but we're trying to do what we can about
this problem.

> As it happens, there are 2 easy global cleanups that I would desperately
> *like* to see because it will solve significant problems for me with
> rxk5.
>
> /1/ remove trailing white space from lines.
>
> /2/ get rid of "register" declarations.
>
> Ideally, also, run everything through "unexpand -a".

Global trailing whitespace removal, deciding what we're going to do about
tabs and doing something uniform, and removing register globally are three
of the global cleanups we've talked about.  (The fourth, for what it's
worth, is removing all (void) casts of function calls.)

I'm hesitant to do this sort of thing while we know we have huge patches
(yours, OSD) outstanding that need to be digested, although at least the
whitespace parts are amenable to patch -l.

> If it would help, I'm willing to write a perl script that would do
> this. ( Actually, I'm probably stuck writing this even if nobody else
> does value it. )

I'm not sure if the tools are already there, but I suspect a tested tool
to do this would be very gratefully received.

> I am *not* in favour of automatic code formatting.  There's multiple
> places in the afs source where some well meaning person added extra
> comments to make something clearer, and some automatic code formatter
> mangled it.

I think we did one automatic formatting pass back early in the OpenAFS
days.  I certainly have no desire to do it again.

-- 
Russ Allbery (rra@stanford.edu)             <http://www.eyrie.org/~eagle/>