In software development, the code review process stands as a crucial checkpoint for ensuring code quality, fostering collaboration, and promoting knowledge sharing among team members. Despite the importance, many engineers lack a clear mental map of how effective reviews work. This is my attempt to help code reviews and reviewers improve.

Whether you’re steering an open source project or leading full-time a software development team, the key to maximizing productivity lies in efficient code reviews.

  • tatterdemalion@programming.dev
    5·
    2 years ago

    The git push --force is definitely a controversial suggestion, im personally happy with doing that, but I have also personally accidentally force pushed dev/main and seen others do it. Squash on merge is probably a safer habit to have. Also, gitlab and bitbucket both get a bit confused if you forcepush to a branch that is part of a MR.

    You can add branch protections that will prevent you from accidental force pushing to main or dev.

    IMO when I see a PR with “WIP” commits, I just assume that minimal effort was put into keeping the commits organized, and I squash all commits to review the PR. If I see many meaningful commit messages, I will try reviewing one commit at a time.

    When I make a PR, I force push to keep my commits organized. If there are changes I want to make as a result of feedback, and they would create significant churn to rebase all of the patches, then I will apply the feedback in a follow-up commit.

    • CameronDev@programming.dev
      2·
      2 years ago

      I do almost exactly that workflow as well, but I just know its bitten me before. Protecting main/dev is fine, but I have also accidentally force pushed to the wrong branch and wiped out its work as well.

      Muscle memory + Fatigue == Bad time :/