On 11/11/2012 11:58 PM, Chris Lattner wrote:>> Is there a particular sub-system size that makes sense to mark as owned? I have been >> reworking the library call simplification infrastructure recently and will be happy >> to sign up as an owner for that. > > I think that "directory level" is the right granularity. If you're interested in signing > up to maintain and review the whole instcombine library, that would be a great level. To > see what this entails, see: > http://llvm.org/docs/DeveloperPolicy.html#code-ownersI am interested and comfortable with that. I will wait a few days and then update CODE_OWNERS.TXT. -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software
Hi, On 12/11/12 15:11, Meador Inge wrote:> On 11/11/2012 11:58 PM, Chris Lattner wrote: > >>> Is there a particular sub-system size that makes sense to mark as owned? I have been >>> reworking the library call simplification infrastructure recently and will be happy >>> to sign up as an owner for that. >> >> I think that "directory level" is the right granularity. If you're interested in signing >> up to maintain and review the whole instcombine library, that would be a great level. To >> see what this entails, see: >> http://llvm.org/docs/DeveloperPolicy.html#code-owners > > I am interested and comfortable with that. I will wait a few days and then > update CODE_OWNERS.TXT.while I think it's great that Meador is stepping forward to help out here, it does bring up the question of how expert you have to be to become a code owner. As far as I know (please correct me if I'm wrong) Meador isn't the number 1 LLVM instcombine expert, but instead is someone who knows instcombine fairly well and is willing to spend time making sure instcombine is in good shape and stays that way. My take is that you should be able to be a code owner without being the ultimate über hacker for that subsystem, since (1) the über hackers will have their eye on you and will let you know if you get it wrong; and (2) being willing counts for a lot. 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. 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. Maybe Chris should give his take on the whole code owner thing? Ciao, Duncan.
On 12 November 2012 15:35, Duncan Sands <baldrick at free.fr> wrote:> Personally I don't really buy this, but I can understand the concern.+1 I think enthusiasm (paired with good knowledge) counts a lot. -- cheers, --renato http://systemcall.org/
On 11/12/2012 09:35 AM, Duncan Sands wrote:> Hi, > > On 12/11/12 15:11, Meador Inge wrote: >> On 11/11/2012 11:58 PM, Chris Lattner wrote: >> >>>> Is there a particular sub-system size that makes sense to mark as owned? I have been >>>> reworking the library call simplification infrastructure recently and will be happy >>>> to sign up as an owner for that. >>> >>> I think that "directory level" is the right granularity. If you're interested in signing >>> up to maintain and review the whole instcombine library, that would be a great level. To >>> see what this entails, see: >>> http://llvm.org/docs/DeveloperPolicy.html#code-owners >> >> I am interested and comfortable with that. I will wait a few days and then >> update CODE_OWNERS.TXT. > > while I think it's great that Meador is stepping forward to help out here, it > does bring up the question of how expert you have to be to become a code owner. > As far as I know (please correct me if I'm wrong) Meador isn't the number 1 LLVM > instcombine expert, but instead is someone who knows instcombine fairly well > and is willing to spend time making sure instcombine is in good shape and stays > that way. My take is that you should be able to be a code owner without being > the ultimate über hacker for that subsystem, since (1) the über hackers will > have their eye on you and will let you know if you get it wrong; and (2) being > willing counts for a lot. 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.That is definitely a fair assessment. I am not the #1 instcombine super-hacker, but I am willing to keep it in good shape. Even if that means bugging other people :-) Another point to address is whether there can be more than one owner in a particular area. -- Meador Inge CodeSourcery / Mentor Embedded
> 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 7:35 AM, Duncan Sands <baldrick at free.fr> wrote:> Hi, > > > On 12/11/12 15:11, Meador Inge wrote: >> >> On 11/11/2012 11:58 PM, Chris Lattner wrote: >> >>>> Is there a particular sub-system size that makes sense to mark as owned? >>>> I have been >>>> reworking the library call simplification infrastructure recently and >>>> will be happy >>>> to sign up as an owner for that. >>> >>> >>> I think that "directory level" is the right granularity. If you're >>> interested in signing >>> up to maintain and review the whole instcombine library, that would be a >>> great level. To >>> see what this entails, see: >>> http://llvm.org/docs/DeveloperPolicy.html#code-owners >> >> >> I am interested and comfortable with that. I will wait a few days and >> then >> update CODE_OWNERS.TXT. > > > while I think it's great that Meador is stepping forward to help out here, > it > does bring up the question of how expert you have to be to become a code > owner. > As far as I know (please correct me if I'm wrong) Meador isn't the number 1 > LLVM > instcombine expert, but instead is someone who knows instcombine fairly well > and is willing to spend time making sure instcombine is in good shape and > stays > that way. My take is that you should be able to be a code owner without > being > the ultimate über hacker for that subsystem, since (1) the über hackers will > have their eye on you and will let you know if you get it wrong; and (2) > being > willing counts for a lot. 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. > > 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.First: this is a relatively minor concern in the grander scheme of things. I generally don't think it will come up often and will get sorted out correctly if it does. However, when it does come up, without being addressed, i suspect it will take a fair amount of time and energy of the community to sort it all out. So maybe it's worth addressing it now.... Personally, the reason for this concern is the conflation of several independent roles into a single "code ownership" bucket: - What the development policy says: ensuring each patch gets some review. - What the release process follows: code owners get the final say in whether a patch gets cherry picked into a release. - What has happened in practice: code owners make decisions when there are two viable choices with differing pros and cons, but no clear consensus on which direction is preferred. I think your first paragraph's process works very well for the first, "ok" for the second, and not very well for the third. I think for the third point (and to a lesser degree for the second), what you want is *an* expert in the area, not necessarily *the* expert. Interestingly, if we can address the third point, that also solves the only tricky part of the second point: if there is a concern or lack of consensus when bringing a patch into the release, the second point can magically be promoted to the third. I see two easy ways to avoid this issue: 1) Split off the third point from code ownership. Have some other mechanism, which hopefully is not a single-point-of-failure (or success... ;]). 2) Require code owners to be at least one of the "trusted maintainers" of the area they own (to quote the dev policy). I think either one of these would work quite well, and I have no real preference for which we follow. #1 would be most consistent with the wording of the documentation, and #2 would be most consistent with existing precedent and activities of the community.
On Nov 12, 2012, at 7:35 AM, Duncan Sands <baldrick at free.fr> wrote:> On 12/11/12 15:11, Meador Inge wrote: >> On 11/11/2012 11:58 PM, Chris Lattner wrote: >> >>>> Is there a particular sub-system size that makes sense to mark as owned? I have been >>>> reworking the library call simplification infrastructure recently and will be happy >>>> to sign up as an owner for that. >>> >>> I think that "directory level" is the right granularity. If you're interested in signing >>> up to maintain and review the whole instcombine library, that would be a great level. To >>> see what this entails, see: >>> http://llvm.org/docs/DeveloperPolicy.html#code-owners >> >> I am interested and comfortable with that. I will wait a few days and then >> update CODE_OWNERS.TXT. > > while I think it's great that Meador is stepping forward to help out here, it > does bring up the question of how expert you have to be to become a code owner. > As far as I know (please correct me if I'm wrong) Meador isn't the number 1 LLVM > instcombine expert, but instead is someone who knows instcombine fairly well > and is willing to spend time making sure instcombine is in good shape and stays > that way.Right.> My take is that you should be able to be a code owner without being > the ultimate über hacker for that subsystem, since (1) the über hackers will > have their eye on you and will let you know if you get it wrong; and (2) being > willing counts for a lot. 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.I agree. I also think that "knowing your own limitations" is an important part of the job. If Meador (or any other code owner) doesn't feel comfortable with something, feels like a patch is questionable, or is outside the domain of his expertise, then he should start a discussion about the topic. He isn't the one that has to "know everything" so much as make sure that InstCombine is getting love, the patches are getting attention, and that people aren't doing things that are against the architecture of Instcombine. Also, having a specific code owner doesn't mean that everyone else should stop paying attention to the patches :). Finally, if someone isn't working out well as a code owner, I'm confident that it will be recognized and they will gracefully bow out of the position.> 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 really see it this way. If a code owner wants "thing A" to happen and much of the community is strongly in favor of "thing B" happening, then it should be discussed until there is some agreement about what should happen. If no resolution can be had, I am happy to personally help resolve the dispute. -Chris