Settings

Theme

Send small PRs (Google eng-practices)

google.github.io

25 points by amir-h 4 years ago · 5 comments

Reader

tharkun__ 4 years ago

I really like this. We try for small PRs as well. Has all of the benefits listed!

What they aren't good at is to show the overall intent of the whole change. I.e. what I've found is that sometimes an individual PR is easy to review and approve because it seems innocuous and totally fine. But in the overall grand scheme of things I'd have "questions". I might even question the overall concept of the whole thing but I don't have the full picture and what seems innocent as a small PR is actually going towards a grand vision I can't support.

I won't deny that I've used the same technique to get otherwise probably more controversially discussed stuff through ;)

say_it_as_it_is 4 years ago

Google ought to spend effort on understanding and then explaining how to chunk large pieces of work that don't seem to qualify. Recommending that an engineer asks their peers for help shows that management also has no idea how to chunk the work.

the_arun 4 years ago

What is a CL?

  • p_l 4 years ago

    Change list, essentially equivalent to a PR. Gerrit also uses them so it's a good place to check for the differences (squashing of commits into single CL for example)

  • amir-hOP 4 years ago

    CL stands for change list.

    It's the unit of change that goes through code review (so for the context of that document I think the best translation to the git world is PR).

    Though it's not exactly a PR, a CL is also a single change in the change history (so you could also say that a CL is equivalent to a commit)

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection