[underdeserver]

I'm a strong believer in fast reviews.

I get into deep working mode for 3 hours a day total, on a good day. The rest is meetings, daily sync, coffee, lunch, "hey can you look at something", hallway conversations, emails, my own inability to concentrate when I'm not feeling it.

I've been in this industry for coming up on ten years. None of this is going to change, unless I become an academic or a hermit.

I don't get to do deep work, at least I'm going to unblock other people as fast as possible. That means out of an 8-hour work day, you're going to get a review from me within half an hour in most cases.

I've been on the team for years and I have write access. I WILL merge your change if you pass my review.

I find that this has immense benefits:

1) People just do things. They don't schedule design meetings, get approvals, get consensus. You know why? Because if someone has a good reason why the commit wasn't a good idea, we roll back. No harm, no foul. And guess what? It happens once in 100 commits. (If it's something truly complex, you do get a design doc approved first. But then the review is about making sure your code is correct, matches your design and our style/testing requirements, not whether it's the right thing to do.)

2) People write good commit messages. If your commit message isn't in the following format:

  Push foos in bar order instead of baz order.

  Following discussion with johnsmith, benchmark 
  (http://<shorturl>) shows 12% improvement in the hot 
  frobnication flow.

  Ticket: http://tickets/<ticket_number>

I'm sending it back to you. Since I merge most of my team's code most commits look like that.

3) People write small commits. Got a bigger change? I'll ask you to split it up (without breaking the build if we ship a version between commits). People don't push back on that, because they know it's not going to add a lot of overhead.

4) In the same spirit, people don't push back on changes I request - unless it's for a good reason. Discussions are on-point. When changes are made you get back approval half an hour later. No background psychological pressure of "I wanted to get this in today and I don't want to have to restore context tomorrow morning".

The velocity you reach is amazing. True serendipity. Unless you're consistently able to get full days of deep work in, I suggest you try it.

(edited for formatting)

[wantsanagent]

Variation on 2 - We don't care about commit messages at all. All PRs must have good descriptions which use a company-wide template. But individual commits get squashed and their commit messages entirely replaced by the PR template contents.

So my commit history is

"fump" "GAH" "I think this works?"

etc. but all changes have a description of the issue (and link back to the ticket) the solution described in some detail.

[sa46]

Is there any tooling you use to squash the commits and use the PR message?

[CraftThatBlock]

Github has a "squash and merge" option during merging. You can disable the other options (merge, rebase) in the repo settings.

I've worked with squash and merge almost everywhere and it's great. Keeps history clean (1 commit = 1 ticket, usually), and links back to the PR with discussions and context.

[gtirloni]

Unless your tickets are tiny, the single commit will have multiple functional changes and be harder to rollback.

I squash my commits into functionally independent units so they make sense and can be rolled back easily. Rebase FTW.

[KronisLV]

A sibling comment mentioned GitHub, I'll also add GitLab here as well, which allows you to do the same thing in their merge requests (the equivalent of pull requests).

Just checked, it doesn't seem that my self-hosted Gitea instance allows for that, so neither does Gogs (or maybe I just can't find the option in the UI).

[amadvance]

> I WILL merge your change if you pass my review.

Doing that, you remove the possibility at the submitter of a final check before the merge. Especially if the merge happens on another day, rechecking with a clean mind helps to find missed things.

Then someone does that to my changes, they get a kind message to not do this anymore.

[underdeserver]

Of course, this is only if the submitter explicitly agrees to that.

[mike_d]

> 2) People write good commit messages. If your commit message isn't in the following format: [snip] I'm sending it back to you.

Try reviewing commits without reading any associated messaging, or having your team submit complex changes with no message. You have to engage your brain to understand what the code is doing and what is being changed, you'll have a better understanding of if the comments are useful or insufficient, and most importantly you don't already have a bias that the code does what it says. You may find that when you really look at it foos are getting pushed in qux order, or that the benchmark was calling a different function.

[rlkf]

My takeaway from your comment is that one should read the code first, and then see if the comments match your understanding of it afterwards, NOT that one should not write any comments.