top | item 38830813

(no title)

pjbster | 2 years ago

It feels like there's a tiny bit of conflicting advice in this. There's the "not rocket science" rule which results in "no CI-failing commits on main" and then there's this:

> Third, our review process is backwards. Review is done before code gets into main, but that’s inefficient for most of the non-mission critical projects out there. A better approach is to optimistically merge most changes as soon as not-rocket-science allows it, and then later review the code in situ, in the main branch.

But the tip about adding a failing test as a separate commit on the feature branch wouldn't survive a merge and it wouldn't live long enough to be reviewed either.

I like most of the advice in this article but this review one is giving me pause.

discuss

order

epage|2 years ago

The idea is one commit adds the test and another fixes it.

sam_bristow|2 years ago

Also, the commit adding the failing test isn't breaking anything, its demonstrating the already broken nature of the existing codebase.