[HN Gopher] The Perfect Commit
___________________________________________________________________
The Perfect Commit
Author : todsacerdoti
Score : 52 points
Date : 2022-10-29 20:52 UTC (2 hours ago)
(HTM) web link (simonwillison.net)
(TXT) w3m dump (simonwillison.net)
| wellpast wrote:
| Going to express a counterpoint.
|
| Maintainable/malleable software is mostly to due with the
| architecture of the code at any given point in time, not the
| sequence of commits that got it there. (I'll take all N,000
| commits collapsed into one with a message "Stuff" if the code is
| well-articulated than a series of perfect commits that culminate
| into a highly coupled volatile system.)
|
| Furthermore, I'll posit that these "perfect commits" have a
| tendency to trend toward overall bad code architecture. Primarily
| because the attention flashlight is being shined on the wrong
| then (the delta as opposed to the resultant codebase). In the
| course of working on a system I'll often earmark code
| decoupling/cleanup as part of a commit. Or move to get cleanup in
| quickly so that my general feature/value delivery is achieved.
| But processes that focus attention on commits (like this "perfect
| commit" and also formal code review processes) discourage this
| kind of continuous cleanup/refactoring that is the only way to
| get to good code.
| Waterluvian wrote:
| Raise your hand if commits are mainly just a way to save
| progress. It can't just be me.
|
| I appreciate that needs differ between projects. In some public
| project I'd be more disciplined. But at work I just commit like a
| madman and collapse them all automatically upon merge.
| ISL wrote:
| The key world is "mainly". If things go wrong, you haven't just
| saved progress, but also saved history. That distinction is
| powerful.
| Waterluvian wrote:
| Indeed. A clean history can be helpful. And this is certainly
| context specific. In my experience it hasn't been as useful
| as the cost to do so, so I've relaxed a bit on making my
| commits carefully.
| [deleted]
| harryvederci wrote:
| For work it's different, but for my side project:
| $ git log --pretty=oneline | wc -l # Total amount of commits.
| 1369 $ git log | grep '^ \.$' | wc -l # Count
| commits with a message of "." 902
| [deleted]
| tkiolp4 wrote:
| Here. I have a colleague obsessed with the "perfect" commit,
| with rebase instead of fast forward merges and what not. Can't
| stand it. I do care about writing a good summary commit title
| that includes the Jira issue number that links to a well
| description of the feature/issue at hand.
| vsviridov wrote:
| "Link to issue for context"
|
| Just no...
|
| Why would you decouple the change from its context? Just add all
| the relevant information to the commit message itself, so it's
| preserved.
|
| I follow my friend's approach which works really well.
|
| https://yrashk.medium.com/solving-problems-one-commit-at-a-t...
|
| Too many a times I had to do code archeology to encounter
| archived Jira or some old space I don't have permissions for,
| that was originally migrated from GitHub issues and all the
| ticket numbers are in complete mismatch.
| simonw wrote:
| You can't put screenshots in a commit message.
|
| You also can't update a commit message after you've pushed it.
|
| Agreed though that you shouldn't work like this if you can't
| trust your organization to reliably archive your issue threads!
| Salgat wrote:
| Both Github and Bitbucket support all of that in the PR, so
| if necessary, update your PR with additional
| comments/information. When I'm looking at a change, I always
| go back to the PR anyways since it provides discussion
| relevant to the change.
| simonw wrote:
| Yeah I sometimes use a PR in place of an issue thread for
| this kind of thing, because PRs support the exact same
| comment system that I find so valuable in issues.
| sjansen wrote:
| > Why would you decouple the change from its context?
|
| The issue is the context.
|
| At work, we require a link to the issue/story because it makes
| SOC2 audits much easier. The commit message is a great place to
| explain the chosen implementation, but that's not all there is
| to software development.
|
| It rarely makes sense to repeat in commits how the issue was
| reported, or why it was given a specific priority. And it can't
| track anything after merge, like how the change was tested, or
| whether the change was communicated to users.
|
| Maybe you don't have to deal with auditors. Maybe your work has
| different processes to keep them happy. But as far as I'm
| concerned, is it important to link the issue for context? Yes!
| Just yes.
| [deleted]
| [deleted]
| thunky wrote:
| > Sometimes I'll even open an issue seconds before writing the
| commit message, just to give myself something I can link to from
| the commit itself!
|
| IMO generally an issue should precede an implementation - it
| should track _future_ work to completion, not work that is
| already complete.
|
| Creating a new issue and then immediately closing it is an anti-
| pattern, in the same way that creating a pull request and
| immediately merging it is.
| simonw wrote:
| Most of my work has an issue created at the start of the
| development process and closed with the final commit.
|
| But occasionally I'll miss that step - maybe I dived straight
| in to fix a new bug without taking the time to open the issue
| first.
|
| This are the cases where I'll open an issue at the last
| possible moment - on the basis that an open-and-shut linked
| issue is still better than a bug fix with no associated issue
| at all.
| thunky wrote:
| > an open-and-shut linked issue is still better than a bug
| fix with no associated issue at all
|
| It sounds like your issue tracker is a part time
| documentation repository.
|
| I think I view this as an anti-pattern because I look at the
| issue tracker as a shared collaboration tool for planning &
| tracking, not as a tool to document all code changes.
|
| For example, if I were solo I'd probably never create an
| issue (or a pull request).
| simonw wrote:
| > It sounds like your issue tracker is a part time
| documentation repository.
|
| Yeah, it's exactly that.
|
| I'm planning to write more about this. Basically I've
| started thinking of issues as "temporaral documentation" -
| documentation with an attached timestamp that was known to
| be accurate only at the point when it was written.
|
| A challenge with regular documentation is that when you
| first write it you are making a commitment to keep it up-
| to-date in the future. A project with thousands of pages of
| outdated documentation isn't much better than a project
| with no documentation at all (it might even be worse).
|
| But a project with thousands of pages of timestamped issue
| comments feels different to me: there's no expectation that
| those will be updated in the future, but they still offer
| enormous value in terms of capturing the decisions that
| lead to the present day.
|
| So yes: I do consider my issues as a form of documentation.
| They're not a replacement for traditional documentation,
| but they're a valuable extension to it.
| pkrumins wrote:
| My definition of a perfect commit is any commit that goes to
| production because only shipping matters.
| mejutoco wrote:
| Instead of downvoting I am giving my opinion: if you really
| believed this you would not be using version control.
|
| P.S. I am assume you do, since you mention commits.
| Supermancho wrote:
| > if you really believed this you would not be using version
| control. > P.S. I am assume you do, since you mention
| commits.
|
| That's circular logic, at best. This is about the label given
| to "perfect" commit(s), for which someone has made an
| arbitrary qualitative list of attributes. The GP has a
| different qualitative list.
| [deleted]
| fernandotakai wrote:
| at the end of the day, i work to make customers productive and
| happy, so the perfect commit is the one that moves the product
| towards that goal.
| Salgat wrote:
| Which includes a sensible (but not excessive) approach to
| avoiding technical debt that inhibits future functionality
| and feature generation.
| simonw wrote:
| Probably should have mentioned that in the post: a benefit of
| working this way - in particular bundling tests in the same
| commit as your implementation - is that it makes it much safer
| to implement continuous deployment.
|
| Many of my projects that use this style of commits are
| configured so that code is deployed to production every time a
| commit passes CI.
| smcleod wrote:
| Only shipping matters - _to you_ (and your product owner - but
| only in the short term), in the medium to long term shipping
| matters (perhaps most), but absolutely not only.
|
| Maintainability matters. The happiness and health of your peers
| matters. Security matters. Ethics matters.
|
| The culture of 'only shipping matters' is reflective of the
| exploitational aspirations of our capitalistic societies, it
| devalues the bigger picture.
|
| If you only care about getting your code out the door - you're
| part of the problem.
| halayli wrote:
| it's easy to ship but it's much harder to continue shipping
| quality software with this approach. The more information in a
| commit the easier it becomes for newcomers to understand the
| context of what they are about to change. I sometimes end up
| reading the comments of an old PR to understand why a certain
| decision was made when it could have been in the commit.
|
| It gets worse when you have people in critical positions
| backing bad software engineering practices under business
| related excuses as this proves nothing but short sighted and
| the fact that they aren't a good fit for the job as technical
| debt is going to be waiting around the corner for payback.
| quickthrower2 wrote:
| That is the perfect merge commit onto trunk/main. Branch commits
| can be chaotic if needed. The commit is then the unit of
| "walkawayability" a checkpoint where if thing get screwy they are
| happy to revert to.
| thom wrote:
| Somehow I'm really wary of all the energy that goes into source
| control workflows and commit messages. It's all meta-information
| and the only time it really matters is when something has gone
| catastrophically wrong. If you genuinely have to go looking at
| the history of a codebase to find out why something weird has
| happened, that feels like a codebase crying out for a better
| structure. New features shouldn't be major surgery, they should
| be instances of established abstractions. I'm very interested in
| languages that make that modularity easier but if you can't
| delete your entire Git history with no negative consequences that
| just seems like a disaster to me, on both an individual and
| industry-wide scale.
| cratermoon wrote:
| > the only time it really matters is when something has gone
| catastrophically wrong
|
| Or in other words, it matters most when it's most needed and
| the recovery process is glad to have them.
|
| Think about the airline industry and how many times the causes
| of an accident have been found in the months-old maintenance
| records of an aircraft, or in an examination of pilot training
| from last year.
|
| Now imagine throwing away all the information that could have
| led to finding a cause and putting in place mechanisms and
| practices to prevent a recurrence. This is how the software
| industry continues to stumble over the same problems that were
| known and described in the 70s.
| willm wrote:
| Great article. I'd like to quote it in a GitHub issue template.
|
| I struggle with "a single focussed change". As my coworkers will
| attest. When working on new features rather than bugs, I find it
| hard to say when one feature ends and another begins. Something I
| need to work on!
| quickthrower2 wrote:
| Try to say it before coding, and probably the team should help
| with chunking it down.
|
| Sometimes a thing cannot be chunked down more without silly
| inconvenience so keep that in mind.
| simonw wrote:
| One thing that works for me here is to try and split the work
| into changes that could be shipped to production independently
| of each other.
| ivix wrote:
| When in a serverless environment it's easy to get into a
| situation where you need to commit to deploy and test anything.
| Then git squash is essential, unless you're the only developer.
| g051051 wrote:
| > Our job as software engineers is not to write new software:
| it's to apply changes to existing software.
|
| Well, I could stop reading right there.
| simonw wrote:
| I probably should have expanded that point: I was trying to be
| pithy.
|
| What I meant is that we spend 95% of our time making changes to
| software - adding new features, fixing bugs etc.
|
| Starting a new project from scratch is something we might do a
| few times a year at most, but changing an existing project is a
| thing we do every working day.
| g051051 wrote:
| Adding new features is writing new software in my book.
| unionpivo wrote:
| Most developers add new features by modifying/adding to an
| existing code/repo most of the time.
|
| Hell even when you create new project, a lot of development
| environments provide basic scaffolding, that you then
| modify (or sometimes remove 95% of).
| khpart wrote:
| It's Python, which is a churnfest by design. Other communities
| have this problem to a lesser degree.
| ecshafer wrote:
| Commit basically doesn't matter to me, I think in the terms of
| PR. I can make 100 commits or 1, it doesn't matter because when I
| merge I squash them into a single commit that encompasses
| everything. There are some exceptions, as splitting a large merge
| into several smaller merges can help a lot, for example adding a
| database column in a pr, then adding in a separate merge usage of
| that column.
|
| In terms of commits, the commit message should be a good summary
| of what is there. In terms of a PR there should be several
| things. One is obviously tests present. The next is a what, why
| and a how, which should be explanations of why you are doing it,
| what the purpose is, how you are doing. Finally I think there
| should be a descsription of how this should be tested manually.
| simonw wrote:
| Totally agree - if I'm working on a larger change I'll usually
| do that in a branch, which ends up as a PR which I then squash-
| merge into a "perfect commit" as described in the article.
| neon_electro wrote:
| This is why I now love GitHub's "squash & merge"; it
| accomplishes so much of the ideals laid out in this "perfect
| commit" article.
| rektide wrote:
| Its tragic how either/or git is.
|
| Both are great in different ways. If Im trying to understand a
| complex tricky bit of code, a long string of small commits is
| ideal. If Im browsing a repo, large squashed commits or PRs are
| better.
|
| It's be great to have better heirarchy, to get both.
| drdec wrote:
| You can have both if you keep the PR branches
| Twisol wrote:
| This is why I follow a rebase-then-merge approach. The
| commit graph looks like this: | *
| |\ | * | | | * |/ *
| |\ | * |/ |
|
| Each PR is neatly delineated, and I can quickly follow
| things at coarse-grained level (follow only the left
| parents; there's even a `git log` flag for this) or at a
| fine-grained level.
|
| This isn't just pretty -- this has been _actively_ helpful
| to me many, many times. I can `git bisect` into a merged PR
| to more precisely identify where an issue arose, and `git
| rebase` onto intermediate commits instead of jumping all
| the way to the end if I find a whole PR to be problematic
| to rebase past in one go.
| vvillena wrote:
| I also try to adopt this approach whenever possible. The
| 'git log' flag you mention is '--first-parent'.
| dec0dedab0de wrote:
| I leave all the commits in without squashing, but filter to
| only show merge commits
| J-_-_b wrote:
| here's 95% of all of my commit messages: "wip"
| ChrisMarshallNY wrote:
| I will often paste a line from the CHANGELOG.md into the
| commit.
|
| We used to use Perforce, which forced you to write a commit
| message. One of my employees used to write "asdf". It got a bit
| old. I didn't care that much, myself, as he was responsible for
| maintaining his own code, and he did fine, there.
|
| The Japanese hated it, though, and it probably played a big
| part in his getting laid off. I learned a big lesson, there,
| and insisted on my employees being more circumspect, after
| that.
| WesolyKubeczek wrote:
| My perfect commits have message ,,WIP on stuff", include changes
| to code coupled with formatting changes, have zero tests, nada
| documentation, and no links to issues.
|
| But they are perfect nevertheless, because I judge them so. And
| because the code works.
|
| Also, because I get to get out more.
| airstrike wrote:
| I feel seen
| js2 wrote:
| I wanted to get out today, but instead I spent two hours
| debugging an issue because I was dealing with code from someone
| who thought like you who didn't think documentation, comments,
| meaningful commit messages, or tests were a good use of their
| time.
|
| I don't care what you do with your code, but as soon as someone
| else inherits it, you're a sociopath if you didn't document
| anything along the way.
|
| And btw, the person who inherits that code may be your future
| self, and you'll thank your past self for writing down why you
| did something the way you did.
| simonw wrote:
| This post was extracted from a talk I gave at DjangoCon a few
| weeks ago, which was about how this kind of workflow has
| /increased/ my personal productivity by a material amount.
| https://github.com/simonw/djangocon-2022-productivity
|
| So I get to get out more because I'm doing this!
| WesolyKubeczek wrote:
| You may think so ;)
| js2 wrote:
| Perfect commits are a bulwark against technical debt:
|
| https://www.infoq.com/articles/business-impact-code-quality/
|
| https://news.ycombinator.com/item?id=33372016
|
| The extra time spent today on crafting a perfect commit today
| will save 10x that time in the future. Your future self will
| thank you, and if not your future self, then I will, when I
| inherit your code.
| psychoslave wrote:
| I very rarely check the repository history, even less to find
| an answer in how some buggy behavior was introduced.
|
| At most I'll git blame, search the issue repository and see if
| I can chat with the concerned people.
|
| Understanding the current relevant code and what need to be
| done is basically always sufficient to do the job. Digging into
| history, while potentially very interesting, never helped to
| reduce the time to resolve anything in my experience. If the
| code is a mess, the history will be exponentially so. If the
| code base is clean and well documented, you might more likely
| have a great set of "perfect commits" that you will never need.
| [deleted]
| quickthrower2 wrote:
| History is useful because blame often get's it wrong. So it
| is a manual blame.
| grogers wrote:
| Writing good code and writing good commits definitely
| clusters together. It's part of a general pride in quality of
| work, organization, and planning.
|
| Going back into the commit history can be useful to figure
| out if something suspicious was introduced intentionally or
| if it was just a mistake. Beyond about 3-6 months, the author
| will generally not have any context to remember small code
| details, so leaving breadcrumbs for your future self and team
| can be nice.
___________________________________________________________________
(page generated 2022-10-29 23:00 UTC)