Hi, Thanks a lot for writing. I like it very much.
Some follow ups / clarifications: Do we differentiate between who PRed? I.e. if it is a committer, do they count for the approval or in that case do we need 2 approvals? I am more favourable to require a second approval as usual, with the idea that I prefer to have more enrollment by the broader community to the review process. OTOH, this may be too much of a burden if committers are the only ones reviewing PRs (i.e. it requires 3 committers: author + 2). Do we treat backward incompatible changes differently? For example, I hardly merge API changes to DataFusion without Andy's approval of the PR. My reasoning is that there may be a larger design/architectural decision for the current API (e.g. matches spark's, enables distributed engines) that I am unaware of when reviewing a specific / narrow PR and I very much appreciate Andy's check that the bigger picture remains consistent. I do not have an opinion here, I just wanted to express this implicit rule that I use. Best, Jorge On Fri, Jan 29, 2021 at 12:39 PM Andrew Lamb <al...@influxdata.com> wrote: > One of the items that we discussed at Wednesday's Rust sync was "what is > the criteria to merge a Rust PR". There was no conclusion at the meeting, > but there was a proposal which we would like to discuss on the mailing > list. > > *Goal*: Keep Arrow Rust PRs flowing in a timely fashion, thereby keeping > velocity high and encouraging additional contributions, while also ensuring > that quality is maintained and all contributors have a chance to weigh in > prior to merge. > > *Proposed Guideline:* (mostly a formalization of what I see happening > already): > > 1. Have 2 approvals prior to merging a PR, with at least one from a > committer > 2. Have been open for several days to allow interested parties time to > comment > 3. All comments have been addressed (including honoring requests for > additional time to review) > > Some flexibility in the rules is likely important: there are different > parts of the code at fairly different levels of maturity, and there are > some parts of the code (e.g. some parts of the parquet code base that don’t > have a large number of reviewers) > > For small changes such as fixing CI, or formatting, less time / fewer > reviews are ok, as determined by the judgement of the committer. > > Please let us know your thoughts, > Andrew >
