Comment by koolba

3 days ago

> When I review code, I like to pull the source branch locally. Then I soft-reset the code to mere base, so that the code looks as if it was written by me.

This is eerily similar to how I review large changes that do not have a clear set of commits. The real problem is working with people that don’t realize that if you don’t break work down into small self contained units, everybody else is going to have to do it individually. Nobody can honestly say they can review tons of diffs to a ton of files and truly understand what they’ve reviewed.

The whole is more than just the sum of the parts.

For those that want an easy button. Here ya go.

``` review () { if [[ -n $(git status -s) ]] then echo 'must start with clean tree!' return 1 fi

        git checkout pristine # a branch that I never commit to
        git rebase origin/master

        branch="$1"
        git branch -D "$branch"
        git checkout "$branch"

        git rebase origin/master
        git reset --soft origin/master
        git reset

        nvim -c ':G' # opens neovim with the fugitive plugin - replace with your favorite editor

        git reset --hard
        git status -s | awk '{ print $2 }' | xargs rm
        git checkout pristine
        git branch -D "$branch"

} ```

Crafting good commits, and good PRs out of those commits is a skill just like how writing good code is. Unfortunately, too many people suck at the former.

  • This does also tie in directly with tickets and the overall workflow the team has. I find this to have a huge effect on how managable PRs are. I feel the majority of devs are quite oblivious to the code they produce, they simply keep coding untill they fill the acceptence criteria. No matter if the result is 200 lines in 1 file, or 1 000 lines in 30 files.

We were a team with highly parallizable data science tasks, we checked out 3 copies of the repo, one for our branch, two for branches where we are the reviewer.