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

Do you ensure your clean commits all pass all CI tests?


I use the same workflow as NateEag and mdavidn. My preference is:

• All commits SHOULD pass all CI tests

Merge commits MUST pass all CI tests

The reason I don't require every commit to pass all tests is to maximize reviewability and logical consistency of commits.

For example, if a file needs to move, and then be modified slightly to function in its new location, I prefer to break that into two commits:

1. A verbatim move operation. This commit will fail CI but can be effortlessly reviewed.

2. Small modifications. This commit will pass CI, and its reviewability is proportional to its size.

In contrast, if you smush those two commits together, it can be hard to see where the meaningful changes occur in one giant diff. (Some tools may be smarter about showing a minimal diff for a move-and-modify commit under limited circumstances, but that doesn't always work.)


I think this is the crucial thing. Commits help with code reviews and they give hints about why some code change happened.


Do you enforce the presence of merge commits, i.e. no-ff?


If I'm enforcing any of this, then I enforce that, yes.

All of these constraints can be enforced programmatically, and if you're going to adopt them at all I think automating them is the way to do it.

Personally, whether I enforce this branching strategy varies from team to team and project to project.

Many projects I've been on had much, much bigger issues to deal with, so something second-order like this never gets to the top of the stack.

That said, it's an approach I like, and I think it yields benefits if you have a team that's bought into it.


Yes, generally. I don't really understand why anyone commits broken junk and then leaves it there.


My places test suite nukes my local development environment for the full integration tests. If I am working on a hairy piece of code I open up a PR and let the CI system farm out the suite to multiple instances so I can get an answer in less than an hour.

The "right" answer is probably to refactor the test suite to be more performant, but that's never going to get approved given the amount of work that would take, and it would take me longer than I plan on being at the company to get it fixed in my spare time.

I do have it passing all tests before I try to merge if that counts?




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

Search: