Hacker News Reader

Comment by generalizations | Hacker News Reader

Honestly, the criticism of the freebsd code review process seems way overblown. To me, it looks like a smaller team that works well together in a less formal manner, and someone exploited that.

They caught it, the person has lost trust, they all moved on. Big whup.

View on HN

Well, it's not the end of the world (and I feel much more sympathy for this guy's poor tenants than I do for pfSense customers), but...

Bad code going unreviewed from a single author into a main branch from which people build production systems is definitely beyond "Big whup" severity. The equivalent would be if one person pushed an unreviewed driver into Fedora Rawhide or an Ubuntu Beta, say. It's a clear violation of the principles behind the service the distro is supposed to be providing for you.

There are a zillion ways to make sure code gets reviewed before merge. Linux does it informally via Signed-off-by headers and a tree structure of trusted maintainers. Services like github provided automated tooling to enforce review. FreeBSD needs to just pick one. It's 2021, for goodness sake. A fixed COMMITTERS list just isn't going to cut it.

>In December 2020, development of the base system migrated from Subversion to Git.

Call me mean, but I don't associate FreeBSD with a project that is quick to adopt modern development practices.

I remember the time that FreeBSD moved from CVS to SVN, and it was hailed as revolutionary. The world was already embracing Git, which, despite its flaws back then, was perceived as a bliss compared to SVN.

That's not mean, it is a compliment. Although the phrase "modern development practices" makes it a loaded statement.

There is much value in not constantly hopping onto every hypetrain that comes along. For a reliable project, I want to see engineering practices that favor remaining on proven stable technologies as long as possible. Let all the hypes die down, don't go down with them.

I don't see how code review is "modern development practices" (in a bad way). It's common sense.

An analogy would be lawyers. They don't represent family because they may overlook something they think is meaningless, but a "fresh pair of eyes" would say is very important. With code, it's the same way: your eyes are biased towards your own code which can cause you to miss a bug.

The items they discuss, like more rigor on MRs, static analysis, etc. seem like no-brainers from five years ago. I'm not knocking them, I don't know the scope of their limitations. And they're doing the improvements now, so good on them. Just seems odd some things weren't being done previously.

My guess, after lurking around the internet for the past few decades and keeping an eye on what BSDs generally do, it's a mix of NIH/people that don't want to change their existing workflows and lack of financial resources. It's hard to say what the percentages are for the two.

Even if you trust your team members to do a good job, another pair of eyes on the code before it goes in rarely hurt. Everybody slips up once in a while and a review has at least a chance of catching the worst mistakes early. Team size doesn't matter. You just need to bring a little discipline to the process.

This weekend I came across an interesting comment on the Netgate WireGuard fiasco from a former FreeBSD Core team member David Chisnall. He does consider it a failure of FreeBSD's process.

https://lobste.rs/s/sh2kcf/buffer_overruns_license_violation...

Reply on news.ycombinator.com