GitHub user JkSelf added a comment to the discussion: Infra: Remove the approval restriction of merging PRs
I believe that a committer should still review and approve the PR in Gluten to ensure the robustness. I understand that although the Spark community does not have a strict rule requiring approval before merging code, not everyone who submits code can merge it directly. It usually requires a senior committer to review the code and +1 before it can be merged. Thanks, Jia Ke From: Jin Chengcheng ***@***.***> Sent: Friday, January 17, 2025 1:59 PM To: apache/incubator-gluten ***@***.***> Cc: Jia, Ke A ***@***.***>; Team mention ***@***.***> Subject: Re: [apache/incubator-gluten] Infra: Remove the approval restriction of merging PRs (Discussion #8556) Yes, I agree with that. I would suggest the committer approves the PR first and then left some time for other committers to notice this PR since we don’t need another committee’s approval. Thanks Chengcheng Jin From: Hongze Zhang ***@***.***<mailto:***@***.***>> Sent: Friday, January 17, 2025 11:17 AM To: apache/incubator-gluten ***@***.***<mailto:***@***.***>> Cc: Jin, Chengcheng ***@***.***<mailto:***@***.***>>; Team mention ***@***.***<mailto:***@***.***>> Subject: [apache/incubator-gluten] Infra: Remove the approval restriction of merging PRs (Discussion #8556) Hi folks, You may notice that in our GitHub repo, at least one approval from committers is needed before a PR is mergeable. This setting is configured at here<https://github.com/apache/incubator-gluten/blob/b09c017dffb4246dc4b29d8cbe6481fa59a50d18/.asf.yaml#L41>. I would like to explore on the possibility of moving this rule. There are a lot of other Apache projects not having this rule but still running perfectly, including the well-known ones like Hadoop, Spark, Arrow, etc. Actually, I found the number of repositories from ASF projects having this restriction is way less than the number of others that don't have this restriction. See my search result: 1. Number of repositories with .asf.yaml defined: ~1100<https://github.com/search?q=org%3Aapache+path%3A%22.asf.yaml%22+%22github%22&type=code&p=5> [1] 2. Number of repositories with .asf.yaml defined with the approval restriction: 228<https://github.com/search?q=org%3Aapache+path%3A%22.asf.yaml%22+%22required_approving_review_count%22&type=code> Hence, I assume in most case a Apache project can run well without this restriction. Removing the restriction doesn't mean PRs can be arbitrarily merged without essential reviews: It's always the committer's duty to consider carefully on whether review is needed for their PR before merging, One may says people make mistakes, I think that's pretty normal however when mistake happens, we can always revert the unwanted changes to fix. As only committers can merge PRs, I think the risk can be controllable given we always carefully nominate committers. We can observe on the overall cost after the rule change is applied. As the change can be experimental, I would recommend to remove the rule and observe on the impact for next several weeks, to see if the new rule is suitable for the community. Do you have any thoughts? cc @apache/gluten-committers<https://github.com/orgs/apache/teams/gluten-committers> [1] Note, ASF doesn't have this much of TLPs. The number includes all the sub-repositories under the TLPs (and incubating ones as well). — Reply to this email directly, view it on GitHub<https://github.com/apache/incubator-gluten/discussions/8556>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AS7NMRV6PYA3WSX2ALQXAIL2LBY3HAVCNFSM6AAAAABVK763HOVHI2DSMVQWIX3LMV43ERDJONRXK43TNFXW4OZXHAZTGNRTGY>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***<mailto:***@***.***>> — Reply to this email directly, view it on GitHub<https://github.com/apache/incubator-gluten/discussions/8556#discussioncomment-11862759>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC3K7WSSSCWV3OPDFGDKSE32LCLY5AVCNFSM6AAAAABVK763HOVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCOBWGI3TKOI>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.******@***.***>> GitHub link: https://github.com/apache/incubator-gluten/discussions/8556#discussioncomment-11862918 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
