wmoustafa commented on code in PR #10780: URL: https://github.com/apache/iceberg/pull/10780#discussion_r1722654876
########## site/docs/contribute.md: ########## @@ -45,6 +45,16 @@ The Iceberg community prefers to receive contributions as [Github pull requests] * If a PR is related to an issue, adding `Closes #1234` in the PR description will automatically close the issue and helps keep the project clean * If a PR is posted for visibility and isn't necessarily ready for review or merging, be sure to convert the PR to a draft +### Merging Pull Requests + +Most pull requests can be merged once a single [committer](https://www.apache.org/foundation/how-it-works/#committers) other than the author is satisfied with the code in the PR. Before merging all review comments should be addressed either by making changes or agreeing the request is out of scope for the PR. For additions to public APIs committers should wait at least 24 hours before merging to ensure there is no additional feedback from members of the community. [Committers are trusted](https://infra.apache.org/new-committers-guide.html#the-committers-way) to act in the best [interest of the project](https://community.apache.org/projectIndependence.html#apache-projects-are-managed-independently). + +Requesting changes on a PR indicates a reviewer believes the PR has merit but still needs issues addressed before merging. If a reviewer believes the change should not be merged at all and there is nothing the author could do to address the reviewers concerns, the reviewer should explicitly state this on the PR. In the rare event that a PR author and reviewers cannot come to a consensus on a PR, the disagreement should be raised to the developer mailing list for further discussion. In this context, a reviewer is anyone leaving comments on the PR including contributors, committers and PMC members. + +There are several exceptions to a single committer being able to merge a PR: Review Comment: My intention was more on the structure. Basically, I think the intended message here is there are changes that can be merged through the PR and the PR review process solely, and other changes require following more formal process, like dev list discussion, IIP, vote etc. I think something like this could make it clearer: ` There are two types of changes: those that can be merged through the regular PR process, and those that require a more formal procedure in addition to the PR process, such as a developer mailing list discussion, an Iceberg Improvement Proposal (IIP), or a vote. ` Then we can describe (1) the regular process and (2) the more formal process required before the regular process. It is essentially the same content but with clearer message that is clearer about the relationship between the processes (e.g., the latter is a pre-requisite of the former in some cases). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org