3 ms·
I mostly agree with you, but I think this might be going a little too far: > You created a new function? That's one commit. Take a moment to explain why this f
by Ajedi32 8y ago
I mostly agree with you, but I think this might be going a little too far:
> You created a new function? That's one commit. Take a moment to explain why this function is going to be useful in future commits.
> You called that new function from several new spots? That's another commit. Explain why each of these calling sites requires this function.
In my opinion each commit should make sense on its own. It doesn't really make sense to create an unused function, so these two changes should really be one commit.
- jordigh 8y agoMaybe. I think both ways can make sense, and if you insist on having the function and the calling sites in the same commit, that can make sense. I think my proposal can also make sense because (1) it splits up the commits into two units that still keeps the codebase in a stable state [this is my rough metric for what "atomic" means] and (2) defining a function requires some independent contemplation about why that function is defined that way. Inserting that function into calling sites can be a logically distinct operation if the calling sites are varied and distinct enough to each have a different reason to now require this function. Thus the two commits can be semantically distinct.
- btschaegg 8y agoI see this the same way. Like the notion that purely cosmedic changes should be committed distinctly in order to not clutter up commits that change behavior (so they are simpler to read), it makes sense to me that one would commit entirely new units of code separately before re-wiring the logic of the existing code accordingly. That way the first commit basically states "I've built this new thing", and I don't have to consider 300 random existing codepoints in understanding what it does.