Hacker News Reader

Comment by cbsmith | Hacker News Reader

Interesting. If before you weren't intending to understand and be able to summarize before, what were you doing when you reviewed code?

View on HN

Writing the summary is a higher bar than just deciding that you could.

Wouldn’t most people just restate the ticket? Or do you mean, they write about the actually implementation. Eg, “this fixes the race condition by removing the timeout and waiting until the initialization task finishes (line #55) before instantiating the chart widget”

The latter would be a very useful code commit message, (the line #55 may be excessive), both for the author and the reviewer.

I long for tickets that descriptive

The ticket would probably just say “this thing doesn’t load correctly sometimes”, my description was for the code review

I agree. I just was curious if that was the only difference. The way it was phrased it sounded like it wasn't.

Here's a contrived example... Let's say a PR arrived that appeared to create a new S3 bucket and a new IAM role with an attached policy that grants read-only permissions to that bucket.

The old me would read it, and write something like "adds S3 bucket and IAM role".

The newer me would write it as:

1. Creates S3 bucket <XYZ> in account <DEF>.

2. Creates IAM role <ABC>, with an attached policy granting read-only access to S3 bucket <XYZ>.

It seems like a small change, but when I'm actually planning to type out those details - I'm more likely to notice typos in the name/account/role/etc, or just outright errors. One of the mental shifts from my old reviews to newer ones is that I open the PR assuming I don't know what it does yet. So I have to read it for myself and interpret it, then summarize it for someone else.

Thanks, this helped clarify for me what your experience was like. It's interesting.

Reply on news.ycombinator.com