Re: [DISCUSS] CODEOWNERS mechanism feedback
We will get these approved as soon as possible. Regards, Nabarun From: Mario Ivanac Sent: Thursday, March 25, 2021 4:17 AM To: dev@geode.apache.org Subject: Odg: [DISCUSS] CODEOWNERS mechanism feedback Hi all, just to remind on this topic. Could we expand list of CODEOWNERS in some areas that are bottleneck. For examples I have 2 PR`s that are waiting on review from WAN area for several weeks: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F6013data=04%7C01%7Cnnag%40vmware.com%7C8118e3d4a0f04955b4c208d8ef7f9bc5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637522678650905214%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=sP9CgS6ZHrz%2BcfslYxFg7y9%2FzNhZLGV%2BsX5ccjO2N2A%3Dreserved=0 https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F6048data=04%7C01%7Cnnag%40vmware.com%7C8118e3d4a0f04955b4c208d8ef7f9bc5%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637522678650905214%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=VH1ntlmmHR2fUaNiylYq6LpmGhOBt7UKXNWVTCn919c%3Dreserved=0 BR, Mario Šalje: Anthony Baker Poslano: 17. ožujka 2021. 18:08 Prima: dev@geode.apache.org Predmet: Re: [DISCUSS] CODEOWNERS mechanism feedback I think for an active, healthy project community we need to balance two things that are somewhat oppositional: make it easy to contribute and ensure the project meets the needs of our users for stability and robust behaviors in the face of failures. > On Mar 17, 2021, at 9:45 AM, Jacob Barrett wrote: > > > My opinion, I think we need some metric by which to quantify the positive > impact against the negative impact otherwise the drag this process has on > progress may be discouraging to people joining or staying with this community. > > -Jake >
Odg: [DISCUSS] CODEOWNERS mechanism feedback
Hi all, just to remind on this topic. Could we expand list of CODEOWNERS in some areas that are bottleneck. For examples I have 2 PR`s that are waiting on review from WAN area for several weeks: https://github.com/apache/geode/pull/6013 https://github.com/apache/geode/pull/6048 BR, Mario Šalje: Anthony Baker Poslano: 17. ožujka 2021. 18:08 Prima: dev@geode.apache.org Predmet: Re: [DISCUSS] CODEOWNERS mechanism feedback I think for an active, healthy project community we need to balance two things that are somewhat oppositional: make it easy to contribute and ensure the project meets the needs of our users for stability and robust behaviors in the face of failures. > On Mar 17, 2021, at 9:45 AM, Jacob Barrett wrote: > > > My opinion, I think we need some metric by which to quantify the positive > impact against the negative impact otherwise the drag this process has on > progress may be discouraging to people joining or staying with this community. > > -Jake >
Re: [DISCUSS] CODEOWNERS mechanism feedback
I think for an active, healthy project community we need to balance two things that are somewhat oppositional: make it easy to contribute and ensure the project meets the needs of our users for stability and robust behaviors in the face of failures. > On Mar 17, 2021, at 9:45 AM, Jacob Barrett wrote: > > > My opinion, I think we need some metric by which to quantify the positive > impact against the negative impact otherwise the drag this process has on > progress may be discouraging to people joining or staying with this community. > > -Jake >
Re: [DISCUSS] CODEOWNERS mechanism feedback
> On Mar 17, 2021, at 9:38 AM, Nabarun Nag wrote: > > "the review process is taking longer now. " > I agree that the review process is taking a bit longer, but that is the price > I believe needs to be paid to improve the probability of good quality code is > being merged to Geode. More eyes on the code mean that more issues may be > detected. Our goal should not be to merge code as fast as possible but > well-vetted code goes into the codebase. How are we going to measure this? By the time it takes between cutting a release branch and actual release? By the number of blockers to a release between releases? To the number of releases that have to be pulled for major issues? My opinion, I think we need some metric by which to quantify the positive impact against the negative impact otherwise the drag this process has on progress may be discouraging to people joining or staying with this community. -Jake
Re: [DISCUSS] CODEOWNERS mechanism feedback
Hi Alberto! I haven’t looked at PR review throughput metrics. I know that is certainly an interesting measure to keep an eye on w.r.t to the CODEOWNERS / CODEWATCHERS processes. I think another equally interesting metric is the “quality” of PR reviews. This is difficult to measure but you could think of a continuum such as: Level 0: I reviewed the PR and gave you a thumbs up just because Level 1: I reviewed the PR and I like the names and code formatting Level 2: I reviewed the PR and I checked that the change has the intended effect Level 3: I reviewed the PR and I checked that there is sufficient testing so it’s safe to merge Level 4: I reviewed the PR and I believe this change is aligned with the goals of the project and its architecture (Of course these are just examples) If the CODEOWNERS process is moving PR reviews to a higher “level” I could see that the time and effort could increase, particularly for complex and large changes. Overall I would argue that’s a good thing for a project like Geode that has lots of intrinsic complexity in the source code and domain. IMHO, Anthony > On Mar 17, 2021, at 8:46 AM, Alberto Gomez wrote: > > Hi, > > It's been more than two months since the CODEOWNERS file has been in place to > automatically add reviewers to pull requests. While we have seen the great > benefit of having the experts in the matter being automatically assigned as > reviewers to each pull request, I have the feeling that the review process is > taking longer now. Some possible reasons could be: > 1. Some code owners might be getting more reviews than they can cope with and > they have become a bottleneck. > 2. While prior to this change only two approvals were necessary, with the new > process the number of approvals from reviewers required to approve a pull > request can be much higher than two, depending on the number of areas touched > by the PR. > > Again, this might just be my feeling or something incidental and only related > to the pull requests I have been working on. In any case, I would like to > know if others are experiencing this slowdown in the review of their pull > requests. > > Also, I do not know if there are metrics available for the review process. > For example, the average time taken since a pull request is submitted or a > change is made on it until there is a review. Having these types of metrics > would be very useful because they would allow us to evaluate this mechanism > from perspectives other than the quality of the reviews and to propose > corrective actions if necessary. > > Best regards, > > Alberto
Re: [DISCUSS] CODEOWNERS mechanism feedback
Hi Alberto, Here are some of my opinions on this matter. "the review process is taking longer now. " I agree that the review process is taking a bit longer, but that is the price I believe needs to be paid to improve the probability of good quality code is being merged to Geode. More eyes on the code mean that more issues may be detected. Our goal should not be to merge code as fast as possible but well-vetted code goes into the codebase. "Some code owners might be getting more reviews than they can cope with and they have become a bottleneck." Yes, that may be true, committers are free to decrease the load by modifying the CODEOWNERS file through a PR to remove themselves from certain areas. " reviewers required to approve a pull request can be much higher than two, depending on the number of areas touched by the PR." Yes, I agree but it is also necessary if multiple areas of the code are modified. Geode is a complex distributed system, and we have engineers who are experts in certain areas of the code. It is unfair to a single engineer to approve code in areas that are not in their knowledge domain. Also, I think the engineers involved in the approval process are involved in tasks of their own hence it is ok to increase the time window a bit to accommodate them. Also, a pointer, not all code owners in the reviewers' panel in the PR need to approve the PR for it to be merged. Github at the bottom of the PR lists the smallest set of approvals needed to merge the PR. You can email them if it is a priority. In my opinion, quality and correctness are more important than the speed of merging code. We need to reduce the number of bugs/issues that have greatly reduced the process of releasing Apache Geode. Regards Nabarun From: Alberto Gomez Sent: Wednesday, March 17, 2021 8:46 AM To: dev@geode.apache.org Subject: [DISCUSS] CODEOWNERS mechanism feedback Hi, It's been more than two months since the CODEOWNERS file has been in place to automatically add reviewers to pull requests. While we have seen the great benefit of having the experts in the matter being automatically assigned as reviewers to each pull request, I have the feeling that the review process is taking longer now. Some possible reasons could be: 1. Some code owners might be getting more reviews than they can cope with and they have become a bottleneck. 2. While prior to this change only two approvals were necessary, with the new process the number of approvals from reviewers required to approve a pull request can be much higher than two, depending on the number of areas touched by the PR. Again, this might just be my feeling or something incidental and only related to the pull requests I have been working on. In any case, I would like to know if others are experiencing this slowdown in the review of their pull requests. Also, I do not know if there are metrics available for the review process. For example, the average time taken since a pull request is submitted or a change is made on it until there is a review. Having these types of metrics would be very useful because they would allow us to evaluate this mechanism from perspectives other than the quality of the reviews and to propose corrective actions if necessary. Best regards, Alberto
[DISCUSS] CODEOWNERS mechanism feedback
Hi, It's been more than two months since the CODEOWNERS file has been in place to automatically add reviewers to pull requests. While we have seen the great benefit of having the experts in the matter being automatically assigned as reviewers to each pull request, I have the feeling that the review process is taking longer now. Some possible reasons could be: 1. Some code owners might be getting more reviews than they can cope with and they have become a bottleneck. 2. While prior to this change only two approvals were necessary, with the new process the number of approvals from reviewers required to approve a pull request can be much higher than two, depending on the number of areas touched by the PR. Again, this might just be my feeling or something incidental and only related to the pull requests I have been working on. In any case, I would like to know if others are experiencing this slowdown in the review of their pull requests. Also, I do not know if there are metrics available for the review process. For example, the average time taken since a pull request is submitted or a change is made on it until there is a review. Having these types of metrics would be very useful because they would allow us to evaluate this mechanism from perspectives other than the quality of the reviews and to propose corrective actions if necessary. Best regards, Alberto
