1. 1

    Since pretty much all comments seem to be critical, I’m just starting a new thread rather than replying to a certain comment.

    First off: yes, I agree that for open-source, PR is the way to go. One can’t open the repository to complete strangers. I was talking about work, on a closed product, by a relatively small team; we all know each other.

    About master being stable at all times, I maintain that this is a fallacy. Yes, of course master should build and the test suite should run flawlessly. But “tests can only prove the presence of bugs, not their absence” (Dijkstra). Inevitably, bugs will enter master and sometimes (quite often!) the fix is only a single line of code. You should not require a PR and code review for that.

    1. 12

      The vast majority of production outages I’ve seen have started with “this fix is only a single line of code”.

      1. 2

        I think this meme is a cultural signaling mechanism. In my circles a developer would never even say the words “it’s just a single line of code” without using an ironic tone of voice, eye rolling, etc. :)

      2. 9

        You should not require a PR and code review for that.

        You should actually. For one, if it is literally one line of code you probably didn’t write a test to make sure it doesn’t regress again which you should and the reviewer should make you do.

        Another thing is that one line of code changes quite often have unforeseen effects elsewhere. Review can help suss this out.

        Finally, having at least a PR (if not a review) is some automatic documentation that there was an issue and it was fixed.