181–190 of 193 posts
> it is not in general possible to find bugs by examining the code. Oh hell yes it is, at every level of abstraction even. We call those things code smell... A file descriptor that hasn't been closed, a coroutine that hasn't been awaited, a big try/catch block that just falls back to some value without logging the error, wrong type castings, etc. As a general rule: Neither type checker, nor compiler, nor runtime shou…
Earlier quoted context omitted.
You can setup most PR systems to squash on merge using the first commit's message, then enforce that the top commit message is prefixed with a ticket ID. This practice has many benefits: readable git log, easier git bisect for tracking down regressions, it becomes easy to find all commits associated with a block of work, more useful git blame.
I strongly recommend against auto-squashing. It creates large commits without the semantic intent of the author. In a large PR, you want clean, small, semantic commits, which makes it much easier to review and understand.
I think this is pretty clumsily stated. The way I would best summarize what it should be is "The person best suited to address bugs in the code is the author/owner." That's usually some blend of detecting and fixing them. Code review can certainly surface bug-prone patterns being introduced (or extended), and even catch them directly.
But as many of the peer commenters state here, the depth of code review that finds buggy behavior and risky structuring is pretty involved and an expensive use of time. At most places I've reviewed code prior, this was always amortized as cost of doing business. Maybe partly because there wasn't a "theoretically" faster alternative, and maybe because we were trying to avoid 2 steps forward 1 step back.
Now, I think we are struggling with "the clinical trial problem" where there is more pressure to ship using AI but it doesn't actually abridge the QAing and review part. The problem with trying to abridge that part is it just creates backpressure like a spring and then catastrophically explodes 10x worse later on than if it was dealt with directly. It can be very easy to build a circle of pitfalls that were completely unnecessary. I think that automated code review techniques are going to have to evolve to keep up with the new paths that code can be generated through.
And to anyone that has to go through new, arms race enhanced forms of slop cannons -- my heart goes out to you, because the old ones were never particularly pleasant to deal with.
Earlier quoted context omitted.
Totally agree. With the speed at which code can be written and deployed today due to AI, the emphasis should shift onto the review. Does the code actually run properly, are all of our assumptions correct, and are there any unintended side effects? I've found the review and debugging process to be much more time consuming than writing/producing code, and just "praying it works" never ends well.
The problem with this is that so many diffs created by AI are gigantic. Thousands and thousands of lines of code, per PR. Of course this is fine if AI is creating, reviewing AND merging, but it's an exercise in futiliy for human reviewers, as there is no way for them to catch up.
Earlier quoted context omitted.
Not every codebase project etc use such workflow Also such approach doesnt work with bug fixes / regressions
Every team should follow a plan, fine on a side project, but if you work in a large codebase with a bunch of devs, you need to have some sort of workflow to avoid stepping on each other's toes. bug fixes are supposed to be small, contained, if they're rearchitecting the codebase, then they're not _bugs_, but tech improvements, and need to be addressed differently and I agree that this should be flagged in the PR. a P…
For bug fixes usually there are no fancy docs because until fix is found then it isnt known how to fix it (usually)
And after fix is found, there may be improvements applied to it
Code review doesn't have a single purpose. Finding code that is hard to maintain is one of those, and and an important one, but certainly not the only one, and I'm not sure it is even the most important one. Other purposes include: - a safety check to ensure that if a developer (or AI) goes rogue, it is more difficult to merge malicious code - a second perspective from someone who isn't as close to the problem and mi…
1) https://abseil.io/resources/swe-book/html/ch03.html#readabil...
Earlier quoted context omitted.
Apparently the mathematician author doesn't understand the meaning of his own natural language quantifiers. “it is not in general possible to find bugs by examining the code” means “it is not in general possible to find ANY bugs by examining the code”, not “it is not in general possible to find ALL bugs by examining the code”. And the first interpretation is relevant but wrong, whereas the second interpretation is tr…
"in general" in mathematics means "in all cases, without exception", rather different than the normal usage where it means "usually, but not always". If a mathematician using the mathematical sense of the word general says "it is not in general possible to tell if a program will halt by inspecting it", they're talking about the halting problem, even if you've looked at lots of programs where you can tell if they'll h…
I won't respond further.
Earlier quoted context omitted.
"in general" in mathematics means "in all cases, without exception", rather different than the normal usage where it means "usually, but not always". If a mathematician using the mathematical sense of the word general says "it is not in general possible to tell if a program will halt by inspecting it", they're talking about the halting problem, even if you've looked at lots of programs where you can tell if they'll h…
I'm a mathematician AND I understand English. Your statement about the halting problem is not analogous to the one in question and your comment completely ignores my analysis, where I did in fact take "in general" to be universal , not some casual handwavy "usually". Again, as I said , what they apparently meant was (not (for all bugs B, it is possible to find B)), which is analogous to the halting problem statement,…
The best writing on this is the "agent principal-agent" problem, which correctly frames the problem of agents and code review in terms of trust. This is why the solutions for high-trust environments (small teams) and low-trust environments (big companies, open source projects) will be different. https://crawshaw.io/blog/agent-principal-agent