Hacker News new | ask | show | jobs
by fidotron 261 days ago
This will be accompanied by the sort of dev manager that thinks a KPI for "number of PRs merged" won't in any way be gamed or backfire.

I don't know what they're doing where you can do code reviews in 5-10 minutes, but in my decades doing this that only works for absolutely trivial changes.
1 comments
StilesCrisis 261 days ago
The goal here is to make almost all CLs trivial enough that they can be reviewed quickly. You can compose almost any feature out of many small simple changes.
link
fidotron 261 days ago
> You can compose almost any feature out of many small simple changes.

You can?

link
SAI_Peregrinus 261 days ago
Yes, and you can waste a ton of a coworker's time doing it.

Say you're upgrading to a new library, it has a breaking change to an API. First, add `#if`s or similar around each existing change to the API that check for the existing library vs the new version, and error if the new version is found. No behavior change, one line of code, trivial PR. One PR per change. Bug your coworker for each.

Next, add calls to the new API, but don't actually change to use them (the `#if`s won't hit that condition). Another stack of trivial PRs. Your coworker now hates you.

Finally, swap the version over. Hopefully you tested this. Then make a final PR to do the actual upgrade.

For something less trivial than a single breaking upgrade, you can do the same shit. Conditionally compile so that your code doesn't actually get used in the version you do the PR in, you can split out to one PR per character changed! It'll be horrible, everyone will hate you, but you can split changes down to ridiculously small sizes.

link
StilesCrisis 261 days ago
Follow these guidelines and you'll be surprised how far you can go.

https://google.github.io/eng-practices/review/developer/smal...

link
fidotron 261 days ago
So you want code added in 100 line sections behind one feature flag which we then suddenly turn on?

Isn't that exactly how Google's latest big cloud outage happened?

EDIT: referring to https://news.ycombinator.com/item?id=44274563

I should add that the idea complexity relates to LoC is also nonsense. Everyone that's been doing this for a while knows that what kills people are those one line errors which make assumptions about the values they are handling due to the surrounding code.

link