Live data from Hacker News

Improving code review time

engineering.fb.com

31–40 of 233 posts

Re: Improving code review time

#31
post #23
post #21

Earlier quoted context omitted.

I'd say local code quality is generally good but overall it's a big hodge podge of small features duct-taped together, so the whole app becomes a tangled mess. The metric driven culture tends to emphasize impact, not diffs landed.

How would you measure impact?

At first guess, landing/launching a new feature, and then possibly feature adoption.

Re: Improving code review time

#32
post #28
post #21

Earlier quoted context omitted.

I'd say local code quality is generally good but overall it's a big hodge podge of small features duct-taped together, so the whole app becomes a tangled mess. The metric driven culture tends to emphasize impact, not diffs landed.

> I'd say local code quality is generally good Have you ever used facebook.com? At least the frontend is incredibly slow (on a thousands of € machine), so that's not synonym with quality in my experience.

From reading the comment thread I would say the front end would be macro, local would be like a specific function on the front end, like the friends list, that by itself could have "good code".

Re: Improving code review time

#33

Meta: > At Meta we call an individual set of changes made to the codebase a “diff.” GitHub: > Pull request Amazon: > Change Request GitLab: > Merge Request Google: > Changelist Nitpicking, but jesus christ, why can't we stick to a single term?

Is diff the best of them all though, because of the implication?

Diff seems the best because the others describe internal details of the source code management implementation. Pull request, merge request, and changelist especially.

Re: Improving code review time

#34

Meta: > At Meta we call an individual set of changes made to the codebase a “diff.” GitHub: > Pull request Amazon: > Change Request GitLab: > Merge Request Google: > Changelist Nitpicking, but jesus christ, why can't we stick to a single term?

Is diff the best of them all though, because of the implication?

They are all diffs. Simple is better here.

Re: Improving code review time

#35

Meta: > At Meta we call an individual set of changes made to the codebase a “diff.” GitHub: > Pull request Amazon: > Change Request GitLab: > Merge Request Google: > Changelist Nitpicking, but jesus christ, why can't we stick to a single term?

I'd like to propose another to consider "delta"

Re: Improving code review time

#36

Am I reading this right? If the total median time in review is “a few hours”, that means that people are dropping what they’re doing to review the code. That can’t really be a good code review? A context switch alone would be half of that time for me.

Meta had the best code review quality I've seen in my career.

Re: Improving code review time

#37

Meta: > At Meta we call an individual set of changes made to the codebase a “diff.” GitHub: > Pull request Amazon: > Change Request GitLab: > Merge Request Google: > Changelist Nitpicking, but jesus christ, why can't we stick to a single term?

I think this basically doesn’t matter and it’s fine for institutional culture to have its own ingroup jargon.

Trying to standardize for some aesthetic reason wouldn’t add anything

Re: Improving code review time

#38
Here's another factor at Meta that can reduce code review time: Your performance review is based in part (maybe not a large part, but in part) on how many reviews you perform, and how many words you put in to those reviews. edit: In short, people are incentivized to review

Re: Improving code review time

#39

Meta: > At Meta we call an individual set of changes made to the codebase a “diff.” GitHub: > Pull request Amazon: > Change Request GitLab: > Merge Request Google: > Changelist Nitpicking, but jesus christ, why can't we stick to a single term?

diff request makes the most sense to me...

At least that seems descriptive in the context of making a request in a system to get approval to change the code.

Re: Improving code review time

#40
post #6

Earlier quoted context omitted.

Are there companies with a reputation for code quality?

Yes. VMware's ESXi kernel was unbelievably rock-solid compared to other similar systems. Fantastic resilience against even hardware faults. I've heard that there was a culture of "doing things the right way" there. Meanwhile in the same org, the group doing their GUI kept adding band-aids to a broken mess for years.

ESXi is a gem that few appreciate. It's been quietly chugging along as my home server hypervisor since 4.1 (2010). And it's free.
Post reply on HN