I really wish you guys would give me time to review before making these kinds of low-level changes.

I'm sorry that I merged this without your approval. It seems that you're okay with merging changes after other members' approval when it's not about low-level changes, but what draws a line between low-level and high-level? I guess anything that requires #[cfg(target_arch = "...")] is architecture-specific and thus considered low-level?

I think I generally worry about risks caused by leaving PRs unmerged more than other people, e.g. increasing the chance of git conflicts, or blocking changes or investigation that requires other PRs (Arm yjit-metrics this time).

So my default action is to merge a PR once any team member approves it in order to minimize the risks. I believe it's generally more productive to do it and revert changes as needed than waiting for any single person to review everything, especially when it's about fixing a SEGV.

If we have any policy or way of working that isn't compatible with the idea, I'd like to learn about it to avoid making unwanted merges in the future.

I'm sorry that I merged this without your approval. It seems that you're okay with merging changes after other members' approval when it's not about low-level changes, but what draws a line between low-level and high-level? I guess anything that requires #[cfg(target_arch = "...")] is architecture-specific and thus considered low-level?

In general I'd like to review everything if possible, and I try to be pretty fast about it (same day). If it's a change you have high-confidence about being correct it's ok, but for larger PRs I would definitely like to have a look. If I'm on vacation, then obviously I don't want that to stop you from merging PRs.

In this specific case, I think the fix you did is fine. Thank you for finding the bug and implementing a fix.

I think I generally worry about risks caused by leaving PRs unmerged more than other people, e.g. increasing the chance of git conflicts, or blocking changes or investigation that requires other PRs (Arm yjit-metrics this time).

My main worry is that I don't want to merge things too quickly with potential issues that we might fix later and then accrue technical debt. People on the YJIT team are quite skilled and generally do a very good job but sometimes there are fairly obvious issues in PRs and people tend to think "we will come back to this later" but never do. I want to identify those potential problems and raise the bar a little bit, maintain code quality standards, etc.

Understood. Thank you for explaining it. I'll change my default to wait for your review unless you're on a vacation or it's just doc/comment changes.