Hey guys, some of you might have already noticed a delay in merging approved PRs lately. The idea is to ensure more stability and give more chances for additional reviewers to take a look at the proposed changes.
As before, a pull request still needs green CI and two approvals from core team members (one if the author is a member) to be accepted. Once accepted, it is added to the current milestone. But we won’t directly merge it on master. The milestoning signals the PR is scheduled to be merged soon and gives everyone a last chance for comments. The wait time is suppposed to be about a day at minimum but may depend on the gravity of the change. So please don’t ping every core team member 24 hours after the milestone was added. But feel free to bump the PR if a reasonable amount of time has passed without reactions.
This process is a guideline and not a strict rule so there might be occassional exceptions within reason. Urgent infrastructure fixes for example should be expected to skip the line.
I don’t think this matters for the PR template. Let’s not complicate things there. Maybe we could explain the PR lifecycle somewhere (CONTRIBUTING.md ?) and that might be a place to reference this.
Essentially the core team looks for candidates and decides who is going to be invited.
It probably has some effect, but I think this is often overrated. You don’t need merge power for making great contributions, reviewing other’s PRs and taking part in discussions. Engaged non-core members can and do actually do a lot of work.
Still helps to have more core team members, but I don’t think we should expect anyone to do more work just because joining the core team.
Doing proper reviews is unfortunately an incredibly time-consuming task. Your work on the debugger was amazing! But it resulted in a huge PR with many changes to be reviewed and it progressed over time. It’s also a very specific topic. For example I have a very limited understanding of how all this debugging stuff works, so I couldn’t possibly do a substantial review on that.
Honestly, I was quite a bit surprised by how fast it finally got to be merged. Unfortunately, there have been other precedents where it takes much more time to finish some bigger tasks.
I’m not sure what’s the best way to improve on that. More reviews would certainly help and I might remind everyong that not only core team members can do reviews =)
This may seem like a bad thing, but it’s actually really great. Changes to core APIs, the compiler, etc, require a lot of scrutiny because it has a massive influence on the entire ecosystem and can change how everyone works with Crystal. So I’m glad the core team takes the time to think this stuff through that deeply, even when their scrutiny impedes features I’m pushing to improve my own work.
I frequently have to be that person at work that asks the “what about …” questions to try to avoid shooting ourselves in the foot later, so I have a whole lot of appreciation for not only how important that is but also how thankless.
I really like thoroughness of the approval process. I also know that core team is very busy with their day jobs, so I thought that more people will help doing it faster.
