hi Antoine, On Wed, Jan 23, 2019 at 4:35 AM Antoine Pitrou <solip...@pitrou.net> wrote: > > On Tue, 22 Jan 2019 16:57:42 -0600 > Wes McKinney <wesmck...@gmail.com> wrote: > > > > There were 1540 patches merged into the project in 2018 (excluding the > > Parquet merge) -- that's more than 4 patches per day. Evidence > > suggests that the overall patch count for 2019 will be even higher; if > > I had to guess somewhere well over 2000. Out of last year's patches, I > > merged 1028, i.e. 2 out of every 3. If we are to be able to take on > > 2000 or more patches this year, we'll need more help. If you are > > neither a committer nor a PMC member, you can still help with code > > review and discussions to help contributors get their work into > > merge-ready state. > > I generally try to review as many PRs as I feel competent to. > > What should be the guideline when some PRs for other implementations > (such as C#, Java...) are lingering on?
I have generally taken the approach of merging patches when the builds are passing and there has been some code review. In the case of C# as an example, we don't have consistent reviewers so I will generally glance through the code (5 minutes or less) to make sure I see nothing terribly concerning, or to catch other problems like accidental changes to other files in rebase conflicts, or binary files accidentally checked in. It is also helpful to ping people about stale PRs to keep them engaged. > > > I'll do what I have to in order to keep the patches flowing as fast as > > possible into master, but contributors and other maintainers can help > > with the Always Be Closing mindset -- the 80/20 rule or 90/10 rule > > frequently applies. In many cases it is better to merge a patch and > > open up a JIRA for follow up improvements if there is uncertainty > > about whether something is "done". > > I'm quite wary of technical debt (which can quickly plague fast-growing > projects) so I tend to be a bit demanding in my reviews :-) I'm also wary of technical debt -- I am definitely not suggesting to merge patches that you are not comfortable with! =) I have noticed that sometimes patches may get left in a broken state while also falling short of addressing all of the review comments. I would prefer to see a 90% finished patch with a passing build than any kind of broken build. Whether or not that last 10% needs to get done in that patch or in a follow up patch depends. I frequently will step in to "carry" patches when there are small fixes necessary to get a passing build so something can be merged. What "carry" means can depend a lot; e.g. rebasing or fixing lint errors is common. Ideally contributors will take responsibility for getting a patch into a merge-ready state in a timely fashion, but not always. - Wes > > Regards > > Antoine. > >
