• Ephera@lemmy.ml
    link
    fedilink
    English
    arrow-up
    1
    ·
    3 hours ago

    Last year, I was thrown into a senior role and having to do reviews for the first time. We also had to onboard two juniors from another team, who had no routine in structuring commits.

    We didn’t have the capacity to get them up to speed on that, so every time they would submit a badly structured PR, where there was no story. I had to look at each individual line and decide whether it’s good or bad, which is hell.
    Every single time, I would find stuff that was bad and then my feedback was just as useless, because I couldn’t tell them where they took a wrong turn in their story. Instead, I had to basically provide them a diff with the required changes.

    After a few months, we talked about how they should structure commits again (i.e. smallest atomic change) and fuck me, it must have clicked for them at that point, because their next pull request was the first time that I was able to just merge the whole thing as-is.

    I imagine, it helped them a lot, too, to think about what is the smallest atomic change and to be able to test between those changes.
    But yeah, it was wild how different the experience for me was. I could just look at the commits and immediately knew that what they had changed made sense on a fundamental level.

    I had always intended to not be as nitpicky about the details, but if you have to piece together what they changed by looking at the details, that means you have to actively look away. This time around, I had hardly a reason to dive deep into the code, and could rather choose to look at commits, where I knew that the implementation details matter.

    It is possible that I actually did not notice as many bugs, but I went out of that review feeling a lot more confident that the code worked, because I knew that the bigger picture was correct.