pull down to refresh
100 sats \ 24 replies \ @_redacted 20 Aug 2023 \ on: Code review tool recommendations tech
With decade+ on github I’d say that while it doesn’t give you private notes, tagging a line/range with an appropriate comment might work. I’d normally leave something like “this makes sense but doesn’t feel right for some reason and I’d like to think more before approving”.
Are you open to a non-GitHub workflow overall or looking for things in addition to GitHub?
I'm open to anything tbh. We are getting a lot more open source contributions, and if I can review better I can respect the work more.
reply
Overall having comments in a not-github way has a noticeable increase in difficulty for the general-coding public.
It's worth trying to do things more openly/in-github when possible (IMO). Github is quite opinionated though and does take time to adapt workflow to the github-way.
There are a bunch of branch protection rules that can be combined depending on how much you want to block merging before the review is "finished".
reply
deleted by author
reply
Hmmm I'm not sure this is what you're suggesting, but maybe I should save the git diffs to a file locally and write all my notes there!
reply
deleted by author
reply
In some cases the code is fine but I suspect it could have uncaught corner cases or improved but I'm not 100% sure and I don't want to "alert" the author if I'm wrong.
reply
Corner cases are were code reviews are really helpful! It's taken many years to separate "you should have done it how I would have done it" from "this code is ok to be merged". It's helpful to think "does this pr improve the code overall".
Bugs will happen. Keeping the critical sats-related code separate from the rest, + CODEOWNERS will keep the critical sections safer while letting the rest change more quickly.
reply
deleted by author
reply
deleted by author
reply
deleted by author
reply
Noooo I don't want more tools lol. I want one really good one. 😄
reply
deleted by author
reply
How does tig compare to blame view it github? What makes you pick that?
I love love love blame when trying to figure out wtf happened with code. Hampered mostly by ... could-be-improved commit messages / history.
reply
deleted by author
reply
Oh, that looks similar to
git log --graph morereply
Are you looking at the history for your local history or do you pull down the PR ref and look through there?
reply
deleted by author
reply
Are you collaborating on a branch/pr? All pushing to a mainline branch?
(asking as I normally do cicd automation and like knowing more about how folks work)
reply
deleted by author
reply
Branch protection rules, yo!
Github has ... shitty automation around policies to enforce for an org. The rules are wonderfully powerful and while well documented for each option, don't give a good view on how to combine them for a nice safe way to work on code.
deleted by author
reply
Humans are bad machines and the human should set the policy instead via automation.
Code formatters are a great case for just having a machine do it vs letting humans fight in PRs. Have a machine format code, make a status check to prevent bad cases getting through, then require the check to pass before merging.
Too much life has been wasted on debating code formatting in PRs.