I agree with Matt here. We have several examples of projects using CODEOWNERS which are fairly big (Flink, Beam, ...).
We also have examples of projects with "long PR duration" due to too strict policy. I sent another message on this thread about PR review and PPMC. Regards JB On Sun, Oct 30, 2022 at 2:08 PM Matthew Benedict de Detrich <[email protected]> wrote: > > > Pekko is not a mini JSON parsing lib, it's a full fledged application > platform. I don't see why its quality standards should be lower then those > of an average software company. > > I would like to point out that while pay may have disagreements over > CODEOWNERS (or other similar mechanisms) there are projects within Apache > that arguably are big and complex as Akka, one that comes to mind is Flink > which doesn't have any CODEOWNERS file but still shares similar concerns > pointed out in this thread. I also have colleagues that are Flink > committers, I specifically spoke to them on this point and the gist I got > is that not having a CODEOWNERS is in practice (for the problems > being talked about) is a non issue. > > That same colleague pointed out that what it does do however is that it > allows committers to gradually learn about other domains, rather than the > alternative which intuitively often results in delegating a PR to "that > person"/s. With projects that are governed by a company (in Akka's case > Lightbend) this can work because companies have control over hiring a new > "that person"/s and/or retaining "that person"/s but with community run > projects this is a risk. We can definitely have "that person"/s in a > CODEOWNERS file, but if "that person" doesn't have time (which happens for > various reasons) then we end up blocking that domain because no committer > outside of that CODEOWNERS file can review the PR. One can argue at this > point we can find another committer but because no committer outside of the > CODEOWNERS file looked at such PR's we now have a big knowledge gap. > > It might also be useful to actually have a look at how other Apache > projects are run to see how much of a concern is an actual concern and/or > what other techniques are used to address said concerns. > > On Sun, Oct 30, 2022 at 1:28 PM Jean-Luc Deprez <[email protected]> > wrote: > > > TBH, > > > > This is a false contradiction. Apache defines a group of people with a > > larger responsibility then others, being PMC. > > > > The whole system with PMC, IPMC, emphasis on meritocracy, all recognize > > that experience helps judging certain elements. Silly example constant or > > method naming patterns. > > > > If a tool like PR review by code owners is used to maintain a code > > consistency and quality standard. Given that that group will easily exceed > > 30 people, seems like calling that cookie licking is out of place. As long > > as it's understood what the purpose is. > > > > Nor does negate the value of all these proposed automations. > > > > It's the same for commit-then-review vs review-then-commit. > > If you use pull requests, I don't think anyone questions you ensure code > > quality before merge to main. But in a git world that only matters when the > > review proces is ongoing. Once merged, commits are part of the git log that > > makes up main and that that materialized on a branch, has no remaining > > implications. Nor does it block collaborative iterations of improvement > > while still on branch. > > > > I think GitHub PR as a concept is the thing that opens up for OSS > > contribution, I probably never would have gotten to any without. > > > > Pekko is not a mini JSON parsing lib, it's a full fledged application > > platform. I don't see why its quality standards should be lower then those > > of an average software company. > > > > If you're in to the fad of the day, if anything, this is PMC vote shift > > left. > > Code is expected to receive at least 3 PMC votes to get released. Seems > > like 1 PMC approval to get your code in to main is not such a big leap. > > > > I would expect that, the main review would be driven by one or more > > committers and the PMC part is more of a sanity check. > > > > Anyhow, afaik I only get to look at the cookies from the garden window, so > > I'm probably too loud about something I don't have much of a say in anyway. > > > > On Sun, Oct 30, 2022, 09:10 Matthew Benedict de Detrich > > <[email protected]> wrote: > > > > > I don’t think there is any confusion here, when the Apache process talks > > > about cookie licking wrt to code ownership they specifically mention any > > > mechanism which makes the code (directly or indirectly) look like its the > > > sole responsibility of a group of individual/s. This can be done in many > > > ways (such as using the @author tag on source files) and a CODEOWNERS > > file > > > as you describe is also part of that (basically anything that implies > > code > > > ownership in code). https://www.redhat.com/en/blog/dont-lick-cookie is a > > > good article on the topic. > > > > > > Unsurprisingly this is going to be an area of contention especially if we > > > contrast how Akka was developed vs what is considered the Apache way. > > Akka > > > had a set of core engineers working as part of Lightbend with deep and > > > intimate knowledge of the Akka system and while that may work for a > > project > > > that is being maintained and run by a company, as far as I understand > > this > > > is contrary to the “Apache way” which is meant to be community driven. As > > > stated by others there are logistical grounds as to why it can make sense > > > for a project to be governed the way that Akka was, i.e. actors have an > > > extremely high bar both for correctness and for performance and so > > having a > > > set of people with the required expertise that can maintain this bar is > > one > > > way of doing things. > > > > > > I guess we can approach this problem for another angle with the set of > > > tools that we have, for example rather than having a CODEOWNERS file we > > can > > > instead state that any change to core parts of the project (such as > > > actors), needs a PIP (Pekko Improvement Proposal) with a tailored voting > > > system for any design considerations. Parts of this problem can also be > > > solved via tooling, i.e. triggering a benchmark run on CI when PR’s are > > > made against projects to make sure there has been no performance > > > regression, maybe there is also a JVM byte code analysis tool that can > > tell > > > us when the outputted byte code has unintentionally changed when someone > > is > > > doing basic refactoring on critical parts of the project (something like > > > MIMA but checks for any diff in byte code rather than just breaking > > > changes)? In other words extracting the “knowledge” out of a certain set > > of > > > people (i.e. the “code owners”) into both processes and tooling which is > > > visible and transparent to the community. > > > > > > Regards > > > > > > -- > > > Matthew de Detrich > > > Aiven Deutschland GmbH > > > Immanuelkirchstraße 26, 10405 Berlin > > > Amtsgericht Charlottenburg, HRB 209739 B > > > > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > m: +491603708037 > > > w: aiven.io e: [email protected] > > > On 29. Oct 2022, 07:56 +0200, Jean-Luc Deprez <[email protected] > > >, > > > wrote: > > > > Any chance there's some confusion in terminology here. > > > > > > > > When referring to CODEOWNERS, we're talking about the GitHub magic file > > > and > > > > it's functionality. > > > > > > > > That's not about laying claim, but e.g setting required reviewers in a > > > pull > > > > request. > > > > > > > > If you set code owners to repo level for PMC and PR required review > > from > > > > code owners + 2 reviews. > > > > > > > > Anyone can contribute to any part of the code, any committer > > (Maintainer) > > > > can merge PRs, after 2 PR approvals of which at least 1 comes from a > > PMC > > > > member. > > > > > > > > On Fri, Oct 28, 2022, 23:42 Justin Mclean <[email protected]> > > > wrote: > > > > > > > > > Hi, > > > > > > > > > > > It will be a main task to figure out how to evolve such a complex > > > > > > project and how to solve the friction between keeping stability but > > > > > > also figuring out ways and places to evolve. The only way to get > > that > > > > > > done is to find enough shoulders to spread the load. Some mechanism > > > > > > like CODEOWNERS will be needed to figure out who is responsible > > (even > > > > > > if overall ownership is shared, of course) for which part of the > > > code. > > > > > > Saying that everyone is responsible for everything as a whole is > > not > > > > > > realistic. It's also not a realistic expectation for anyone to be > > > able > > > > > > to keep track of everything that might go on in all parts of the > > > code. > > > > > > > > > > The project as a whole is responsible, and that is a shared > > > responsibility > > > > > of everyone. A project must follow the Apache grouping of (P)PMC > > > members, > > > > > committers and contributors/users. This is one of the graduation > > > > > requirements. As I previously said, the use of CODEOWNERs is not > > > > > encouraged. You can set expectations on how people should contribute > > to > > > > > different parts of the project and document them, but all committers > > > will > > > > > have access to the entire codebase. > > > > > > > > > > Kind Regards, > > > > > Justin > > > > > --------------------------------------------------------------------- > > > > > To unsubscribe, e-mail: [email protected] > > > > > For additional commands, e-mail: [email protected] > > > > > > > > > > > > > > > > > > -- > > Matthew de Detrich > > *Aiven Deutschland GmbH* > > Immanuelkirchstraße 26, 10405 Berlin > > Amtsgericht Charlottenburg, HRB 209739 B > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > *m:* +491603708037 > > *w:* aiven.io *e:* [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
