this post was submitted on 19 Feb 2024
53 points (98.2% liked)

Programming

17423 readers
42 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities !webdev@programming.dev

founded 1 year ago
MODERATORS
snowe@programming.dev
Ategon@programming.dev
MaungaHikoi@lemmy.nz
53
How to conduct an effective code review (optimizedbyotto.com)
submitted 9 months ago by otto@programming.dev to c/programming@programming.dev
10 comments fedilink hide all child comments
 

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.

you are viewing a single comment's thread
view the rest of the comments
[–] tatterdemalion@programming.dev 5 points 9 months ago* (last edited 9 months ago) (1 children)

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 points 8 months ago (1 children)

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 :/

[–] tatterdemalion@programming.dev 4 points 8 months ago (2 children)

Pays to know how to recover code from the reflog in these cases.

[–] CameronDev@programming.dev 3 points 8 months ago

Yeah, or sprint to your colleague and ask them to force push their branch again :D

Another tactic for for getting clean git commits is to do all your messy commit work in a scratch branch, and then when your happy, create a new branch, and with meld, organise your changes into complete logical commits. We do that a little bit.

[–] SmartmanApps@programming.dev 2 points 8 months ago (1 children)

Pays to know how to recover code from the reflog in these cases.

At first I read that as re-flog. Is that with a cat o' nine tails? 😂

[–] tatterdemalion@programming.dev 2 points 8 months ago

Your Git requires a regular flagellation to function properly.