It took me a long time to figure out that "this code is shitty" also entails the assumption that I am smarter than the one who wrote it.
If you accept that the other person is also smart, you'll ask "why is this code shitty"? Almost always there's a very good reason behind it.
Often the reason is that a code base evolves across many years where assumptions/invariant change. But you can't always restart from scratch. You need to keep things moving while you evolve, which entails temporary - but potentially long lasting - hacks.
E.g. Geth's data format is bashed for being suboptimal. Well, we know what a better format is, except it breaks sync protocols, so pretty much every other fast syncing client. We've worked hard to find a viable replacement, but we need other clients to jump on to not nuke them.
Another reason is that building something new is significantly harder than to rebuild the same thing. You don't know what the limits are, you don't know what the bottlenecks are, you're shooting in the dark and trying to wing it. That entails a lot of refactors that aren't easy.
E.g. Geth's first "production" version was the Olympic testnet at 8GB. We didn't know what was to come. A client starting now has an enormous dataset to analyse and make fitting designs. Naturally code fitted to the existing chain is cleaner than code evolving with the chain.
Some code may flat out look weird from the outside. It is all too easy to just discard it as the author being an amateur. Generally there's some non-obvious reason why the author chose that. Maybe to get around some language limitation, maybe to get some more juice out of it.
E.g. Go's garbage collector chokes if we store too many plain objects in memory (e.g. 10GB worth of trie nodes). You need to flatten together into byte blobs and get rid of pointers. That from the outside might seem super ugly if you're not familiar with Go's internals.
E.g. Our RLP encoder is nice and generic. Receipt encoding turned out is capped at 10MB/s due to Go's reflection overhead. So now we're generating unrolled decoders. From the outside they look as if the author didn't know reflection, but i turns out it's *exactly* the opposite.
Long story short, there's a reason for everything. The author of a piece of code being a "clueless dimwit" is indeed a possible explanation, but generally it is not the most plausible one. So it might be worthwhile to step back and ask a question first, before going guns blazing.
And yes, there is definitely the occasion where a piece of code is genuinely "just shit". Maybe it was written by someone with less XP; or by someone in a period when they couldn't focus. A nice opportunity to open a PR; but be open to being told it's not "just shit" (scroll up).
Here's to being a bit less sure that we're always right (something I myself am often guilty of) and a bit more open to the work of others.
PS: And no, I'm not talking about that gas limit bribe project. That's an obvious money grab.
• • •
Missing some Tweet in this thread? You can try to
force a refresh
Between the 3 Sept and 10 Sept, secure env vars of *all* public @travisci repositories were injected into PR builds. Signing keys, access creds, API tokens.
Anyone could exfiltrate these and gain lateral movement into 1000s of orgs. #security 1/4
Felix Lange found this on the 7th and we've notified @travisci within the hour. Their only response being "Oops, please rotate the keys", ignoring that *all* their infra was leaking.
Not getting through, we've started reaching out to @github to have Travis blacklisted. 2/4
After 3 days of pressure from multiple projects, @travisci silently patched the issue on the 10th.
No analysis, no security report, no post mortem, not warning any of their users that their secrets might have been stolen. 3/4