Hacker News Reader

Comment by nawgz | Hacker News Reader

If the update is going to touch 27 places at once, how is the developer meant to communicate such a change thru commit messages?

View on HN

I would argue that if you can't communicate the purpose of the change in a simple commit message then you have probably combined too many things into the commit.

Breaking this rule is one of my bad habits and it's annoying for my colleagues to review and increases the risk of including some unintentional change ("I'll just edit this config file for some local testing...") as it gets lost in the noise.

>If the update is going to touch 27 places at once, how is the developer meant to communicate such a change thru commit messages?

I make the commit messages also for myself, how many time you find some code and wonder why is it like that, then you search git and find if lucky a nice message with a ticket id and if you are lucky again the ticket has clear details on what was the issue that was fixed and what behavior was requested.

By grouping the changes logically (by system, context, feature, up to you), as in the example I gave. Sometimes it's as simple as putting backend & frontend changes into separate commits for easier review.

Of course you still need an overarching description in the merge request - should be really easy to write, when you have all the steps taken right in front of you.

This assumes that people work in logical, sequential chunks. I don't. I do a little here, a little there, and then it all starts to form the larger picture.

Not necessarily, I don't think anyone works linearly, but you can still keep your commits organized. I might have 25 dirty files and a bunch of WIP commits; when I'm done I will reset everything, and take the time to stage them one by one (or `git add -p`) into commits that make sense, and will help a reviewer understand what's happening. It's worth the extra few minutes.

For a more concrete example, say you're adding a new screen to a mobile app, I'll probably split into chunks like these:

    - add new `/foo` endpoints to API
    - add `NewScreen` (can be reviewed as a feature, does the screen contain / do what it's suppose to)
    - add `NewScreen` to navigation stack
    - link to `NewScreen` from settings page
    - add tracking for `NewScreen`

Each of these have very different purposes, and I find being able to focus on the different aspects (business logic, infrastructure stuff, analytics) helps a lot. They can also be reverted cleanly if needed without destroying all of the work.

You can still review the entire PR at once, with the option to drill down into individual commits if desired.

Assuming your original commits are atomic but not complete (that is each feature / chunk of work is spread over multiple commits as you changed things up or fixed things you’d forgotten) `rebase -i` might be significantly superior to resetting and redoing everything by hand.

That's why working on a branch, and once ready, taking the time to reorganize your commits into logical steps, is a basic act of kindness towards the people who will review your changes, and towards your own future self.

You don't need to work in logical, sequential chunks. You just should organize the work after the fact before making the PR. I work the same way.

What was changed, why, and link to relevant ticket(s).

Reply on news.ycombinator.com