logoalt Hacker News

regularfrytoday at 10:00 AM1 replyview on HN

So I'd have an immediate problem with the target sequence of commits here. The thing about just getting a "define types" commit is that it shows me nothing about why those types were chosen. I need to flit backwards and forwards in history to see how they hook into the later code. I lose the history of "this type was enough to get us to point A, we needed this other thing to get to point B". But flitting back and forth is exactly what we're trying to avoid here. It feels like we're trying to optimise to One True Clean History, when that can't possibly exist because no two people's idea of "clean" actually matches.

Just give me the PR, don't sweat the individual patches. But maybe also work on not committing your first idea as finished work.


Replies

Zizizizztoday at 11:12 AM

I would imagine why the types were chosen could easily be explained in the commit message. The goal presumably (at least how I do it) is so that if I'm touching quite a lot of the code base, the reviewer has the _option_ of being taken through the narrative of the change like they might explain if they were talking to you. If you don't want to be told what the changes are and how they tie together, then just click on review changes and review it all at once. It's not about the clean history, it's about making the reviewer's life easier with larger features.

The commits might get squashed anyways so the history on main won't necessarily match what's on the feature branch.

You can commit before you raise a pull request, I don't quite understand that point but I might just be missing something about your workflow that's different to mine.

show 1 reply