Re: Consensus on high-level objective of cleanup, refactor or rework patches.

["Carlos O'Donell" <carlos at redhat dot com>]

On Fri, Feb 14, 2020 at 12:23 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Fri, 14 Feb 2020, Carlos O'Donell wrote:
>
> > To that end I propose we approve via consensus the concept of a set of
> > changes and allow the maintainer to commit across the entire code base
> > to implement the concept without having to go through a second round
> > of review. The initial review of the concept is approval enough.
>
> I think it's very rare that this would be appropriate.  Consensus on a
> high-level concept is very different from consensus on the particular
> details of changes - furthermore, various aspects of the details of the
> changes will only emerge as they are implemented, and it's quite possible
> that something seems attractive at the high-level design level but the
> details of the changes result in the whole idea seeming less attractive to
> the community or the design needing to be revisited.

In the cases where implementation causes a deviation from what was
expressed in the initial concept then I expect the trusted developer
to come back to the community to seek consensus around the new
problem.

In some ways a high-level review of a concept is an extension of trust
to the developer. We will follow a consensus driven model but the
consensus is at the architectural level, and trust is extended that
they will carry out an implementation within the shared understanding
we have regarding the code base.

My opinion is that it is costing us too much review time to cleanup
and refactor. These operations should have less implementation review
and more conceptual review.

> Only in some of the most purely mechanical cases (e.g. removal of
> conditional code after we next increase the minimum supported Linux kernel
> version) might the concept make the underlying changes clear enough.  In
> most cases, such a change runs into several special cases that need
> individual attention and review, even if the bulk of the patches are
> mechanical.

For senior developers in the community, like Florian and Adhemerval
what does our review of their implementation details actually gain us?

Their implementations are usually very high quality, but might not
exactly match what I want to see in glibc, but the reality is that
likely either implementation achieves a high degree of success. Only
with the additional review cost. What did that additional delay and
review cost and what do we gain from it?

For example I like Zach's struct-splitting-into-header concept. Should
we allow any patches that do that to just get committed with assumed
consensus? Do we assume this is too disruptive?

Do you have any suggestions for reducing the cost of cleanups or refactoring?

Cheers,
Carlos.