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

Reply via email to