Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

> I agree in general, but running git bisect on individual PR commits is just doing it wrong. There will always be commits that break stuff temporarily.

That's unacceptable in my book. Before submitting any patch set for review, the contributor is responsible for ensuring that the series builds (compiles) at every stage -- at every patch boundary. Specifically so that a later git bisect never fail to build any patch across the series.

This requries the contributor to construct the series from the bottom up, as a graph of dependencies, serialized into a patch set (kind of a "topological sort"). It usually means an entirely separate "second pass" during development, where the already working and tested (test-case-covered) code is reorganized (rebased / reconstructed), just for determining the proper patch boundaries, the patch order, and cleaning up the commit messages. The series of commit messages should read a bit like a math textbook -- start with the basics, then build upon them.

Furthermore, the patch set should preferably also pass the test suite at every stage (i.e., not just build at every stage). Existent code / features should never be functionally regressed, even temporarily. It's possible to write code like this; it just takes a lot more work -- in a way you need to see the future, see the end goal at the beginning. That's why it's usually done with a separate, second pass of development.



I don’t know what world you live in, but I’ve never worked in an organization where more than 1% of the developers would go through all that extra work for every PR.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: