[bookwormAT]

Can someone further explain how the pre commit phase works? I don't get how/why "pre commits" work without feature branches?

How are my changes shared with the reviewer, if I there is no feature branch? Is my local code uploaded to that review tool mentioned in the article? And then what happens if the reviewer requests changes?

I probably did get this completely wrong, so thanks in advance for pushing me in the right direction.

[ridiculous_fish]

You got it right. There is a separate set of tooling layered on top of the VCS, which maintains a sub-history of each commit. The tool (Gerrit, Phabricator, etc.) tracks this relationship between commits, and whatever is eventually merged into the repo.

This architecture assigns each line of code a nested history: the public commit log, and also the sub-history of each commit, which evolved during code review.

IMO it would be better if the code review changes were manifest in the public commit log (e.g. via feature branches), instead of being tracked separately. The code review layers add duplicative complexity.

[tdeck]

Perforce (the origin of Piper) has a concept of changelists. Some changelists are submitted (committed) while others are not. So the review works by uploading your changelist to Perforce, then pointing people to that changelist. It's like an unnamed feature branch that can't easily be rebased off of. Changelists do have a base, and you do a "g4 sync" to essentially rebase off of master. Does that make sense?

[mjw1007]

When people talk about making a single global change to update all clients when they change an interface, are they updating unsubmitted changelists too?

[jsolson]

No.

Unsubmitted changes at Google usually come in one of two flavors, short-lived (abandoned or submitted within a few days) or perpetual. The latter flavor is often for "I think we might want this". It's not uncommon for those to be completely rewritten if they're actually needed. There's usually a preference for submitting useful things (with tests!) and flag gating them to cut down on bitrot.

I have seen exceptions -- I reported a bug in a fiddly bit of epoll-related code and an engineer on my team had a multi-year-old fix -- he hadn't submitted it because he wasn't confident he'd found an actual bug. The final changelist number was more than double the original CL number (unsubmitted changes get re-numbered to fit in sequence when they're submitted -- the original number redirects to the final submitted version in our tooling).

[izacus]

Well, the act of submitting a changelist essentially runs a test suite which requires that the changelist has no merge conflicts with the head and that the relevant tests pass. From that it follows that if someone changes an interface, you'll get either merge conflicts or test failures on your own changelist. Meaning - it's the changelists authors task to sync it up to the current head state so the refactorers won't touch unsubmitted changelists.

It's pretty much the same as GitHub pull requests - the changelists are supposed to be decently short lived and if the master code changes it's up to you to resolve conflicts and get it into a mergeable state again.

[erickj]

every CL (changelist) gets 2 CL numbers, an original (OCL) and committed (CL) number. so CL #s are monotonically increasing, but less than 1/2 are ever actually committed.

when the CL is first uploaded or sent for review the OCL is assigned, the CL number is assigned on commit.