> More power to volunteers! Hopefully if people talk>with each other, help each other out, and maintain a friendly and cooperative >atmosphere then all problems with code ownership will just come out in the wash.+1 I think this is where we want the community to be, and the role of the code owners should hopefully be minimal.>However another take on the whole code owner thing is that the code owner is >the person who has the final say on what goes in and what doesn't, so it is >important for the code owner to be the über guru (and having other people >override their decisions would just undermine their authority and the whole >code owner system). Personally I don't really buy this, but I can understand >the concern.I don't think this is the role of the code owner. The way I understand the developer policy is that the code owner is there to ensure that all patches are reviewed. I would go further and say that the code owner is a fail-safe, which means if the code review falls back on the code owner, the regular review process has failed, and the code owner will have to take some measures (any suggestions?) to make people review patches in a timely manner. It will be too easy for people to stop reviewing, since they now think there is a code owner, which will do all code reviews, and the result might be that person will sooner or later be overwhelmed and drop out. It would perhaps be better to have the code owner be someone that has the knowledge, and cares about a specific component in the context of quality, capability, design and documentation. But it should probably not be the person who is the most frequent contributor to that component (might be difficult to find such a person), since since the reviews may be likely to fall back on the same person, and he/she will have to keep bugging other people about their own patches, or more likely, patches will go in without review if the code owner has a review-after-commit status. - Jan
On Mon, Nov 12, 2012 at 9:22 AM, Jan Sjodin <jan_sjodin at yahoo.com> wrote:>> More power to volunteers! Hopefully if people talk > >>with each other, help each other out, and maintain a friendly and cooperative >>atmosphere then all problems with code ownership will just come out in the wash. > > +1 I think this is where we want the community to be, and the role of the code owners should > hopefully be minimal. > > >>However another take on the whole code owner thing is that the code owner is >>the person who has the final say on what goes in and what doesn't, so it is >>important for the code owner to be the über guru (and having other people >>override their decisions would just undermine their authority and the whole >>code owner system). Personally I don't really buy this, but I can understand >>the concern. > > I don't think this is the role of the code owner. The way I understand the developer policy > is that the code owner is there to ensure that all patches are reviewed. I would go further > and say that the code owner is a fail-safe, which means if the code review falls back on the code > owner, the regular review process has failed,Not exactly - since this includes post commit review too. Since we don't have any positive acknowledgement of post-commit review (when there are no comments being provided) a code owner will generally end up having to perform any cases of post-commit review in there area (since they have no way of knowing that someone else also did so if they provided no feedback). Certainly if a code owner comes up with some way to manage the post-commit review of their trusted reviewers using a shared queue, that could be a way to alleviate some of this load. (& perhaps it goes without saying, but the owner's responsibility isn't just to ensure code gets reviewed, but that it gets reviewed by someone appropriately skilled/knowledgeable in the area)> and the code owner will have to take some measures > (any suggestions?) to make people review patches in a timely manner. It will be too easy for people > to stop reviewing, since they now think there is a code owner, which will do all code reviews, and the > result might be that person will sooner or later be overwhelmed and drop out. > > It would perhaps be better to have the code owner be someone that has the knowledge, and cares > about a specific component in the context of quality, capability, design and documentation. But it should > probably not be the person who is the most frequent contributor to that component (might be difficult to find > such a person), since since the reviews may be likely to fall back on the same person, and he/she will have > to keep bugging other people about their own patches, or more likely, patches will go in without review if > the code owner has a review-after-commit status. > > > - Jan > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> Not exactly - since this includes post commit review too. Since we> don't have any positive acknowledgement of post-commit review (when > there are no comments being provided) a code owner will generally end > up having to perform any cases of post-commit review in there area > (since they have no way of knowing that someone else also did so if > they provided no feedback). > > Certainly if a code owner comes up with some way to manage the > post-commit review of their trusted reviewers using a shared queue, > that could be a way to alleviate some of this load. >I think it would be good policy to have anyone doing a formal (thorough) review send a mail to the code owner with comments (or an OK). Code owners can then organize reviewers the way they want. Keeping track of what has been reviewed is essential, since the majority of commits (I am guessing) are post-commit reviews. Having a person quickly scan over a patch, especially if it ends up being a self-review in case the code owner is committing code, is unlikely to improve things. I don't want a heavy-weight process, but if people are going to come in and drop out etc, there has to be some way of keeping track of things. - Jan