pull down to refresh

I haven't done much code review my career. (I've mostly been a lone wolf.) So I kind of have fresh eyes and the tooling available is disappointing.

For example, when I'm reviewing I'd like to "tag" parts of a diff for further review and write notes for myself, but neither github nor gitlens give me the ability.

So I'm looking for that Good Good code review tooling. Any recommendations?

Why tag and not comment directly? If it's truly raw thoughts, keep the review in draft and continue adding comments while you review. You may find the answer by the time you're done.

If still a "need to check these things" kind of comment, be verbal about it. Nobody in dev will expect you to know every single thing about your codebase. You may even get further analysis and answers from the author. At the very least, you are honest about the delay, and are giving an opportunity for the author to speed it up.

reply
keep the review in draft

Ah, I haven't played much with draft saving on GH. Maybe that will be enough.

Why tag and not comment directly?

I like to progressively enhance any kind of work that I do, e.g. draft 1, draft 2, draft 3, publish1, publish2, publish3. Most code review tools don't seem to support it (at least not in an obvious way).

Perhaps I'm overthinking, but I also like to minimize RTT when communicating about things.

If still a "need to check these things" kind of comment, be verbal about it. Nobody in dev will expect you to know every single thing about your codebase. You may even get further analysis and answers from the author. At the very least, you are honest about the delay, and are giving an opportunity for the author to speed it up.

Great recs.

reply

If you set a "Requires approval" + "Require review from Code Owners" in branch protection, then drafts let you collect your thoughts before publishing them and give security/confidence that things won't be merged prematurely. You can also comment (or not) before approving, and with CODEOWNERS can set where you need to be involved vs someone else.

reply

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?

reply

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

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

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

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

deleted by author

Oh, that looks similar to git log --graph more

reply

I can’t help but feel responsible for this post lol

ETA: You should be able to add comments to the diff under the pending review, and you can choose to remove them before “releasing” the review to the developer.

reply

lol you are. It looks really good by the way. Very tasteful and I'm really impressed with how well you navigated all the quirks of our codebase.

I do have enhancement suggestions though. Would you prefer I simply make the changes myself or would you like me to just suggest them? It's about the same amount of work for me either way, but in the latter case you potentially learn more about the codebase/patterns/deps (assuming my suggestions aren't full of crap ... which is plausible).

reply

Thank you, I appreciate the kind words!

Let's discuss the suggestions on the PR. That way they're documented for others to review later, more potential learning for me, etc.

reply

deleted by author

reply

deleted by author

At work I have used Backlog and Gerrit and would say Gerrit was pretty good. While you are doing comments you can apply it or not to save it. People would reviewed my code were just writing it in the comment: "Note to self: etc". The diff was also good with Gerrit, it highlights changes in a better way than others. I don't know if it changed or not though since now I am also a lone wolf using Gitlab.

reply
100 sats \ 1 reply \ @Joe 26 Aug 2023

Review Board is a web-based, open source tool for code review. To test this code review tool, you can either explore the demo on their website or download and set up the software on your server

The Python programming language and its installers, MySQL or PostgreSQL as a database, and a web server are the prerequisites to run Review Board on a server.

You can integrate Review Board with a wide range of version control systems — Git, Mercurial, CVS, Subversion and Perforce. You can also link Review Board to Amazon S3 for storing screenshots directly in the tool.

reply

We used review board at my old job. I for some reason thought it only supported subversion.

Thanks for the rec!

reply

Just push it to prod

reply
reply

Your frens? :-)

reply

Try the ones on the market