Comment by hnfong
2 years ago
Somewhat unrelated, but it's true that it's sometimes hard to give a reason to reject code that has a "funny smell".
Without going into specifics, there was a case where I reviewed a PR, asking "this isn't usually how people do file operations, are you sure this is really fine?" To address my concerns, they even wrote a specific test program to "prove" that there weren't any problems with the code. I reluctantly approved the PR since I couldn't just ask them to rewrite the whole patch due to just a hunch.
Lo and behold, after a kernel upgrade and file system change, that weird piece of code caused a multi-week panic for the whole team. The extra funny thing is that the test program above made it trivial to confirm and reproduce the issue once we identified this was the cause.
No comments yet
Contribute on Hacker News ↗