git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>,
	Sverre Rabbelier <srabbelier@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH v2 3/4] fast-export: don't handle uninteresting refs
Date: Tue, 30 Oct 2012 18:51:03 -0700	[thread overview]
Message-ID: <20121031015103.GA15167@elie.Belkin> (raw)
In-Reply-To: <CAMP44s0RcbAiUmvGACxO+H-b-anQSPXxUqUuZwYRKWfrpXYeew@mail.gmail.com>

Felipe Contreras wrote:

>                                                    It's not my job to
> explain to you that 'git fast-export' doesn't work this way, you have
> a command line to type those commands and see for yourself if they do
> what you think they do with a vanilla version of git. That's exactly
> what I did, to make sure I'm not using assumptions as basis  for
> arguing, it took me a few minutes.

Well no, when I run "git blame" 10 years down the line and do not
understand what your code is doing, it is not at all reasonable to
expect me to checkout the parent commit, get it to compile with a
modern toolchain, and type those commands for myself.

Instead, the commit message should be self-contained and explain what
the patch does.

That has multiple parts:

 - first, what the current behavior is

 - second, what the intent behind the current behavior is.  This is
   crucial information because presumably we want the change not to
   break that.

 - third, what change the patch makes

 - fourth, what the consequences of that are, in terms of new use
   cases that become possible and old use cases that become less
   convenient

 - fifth, optionally, how the need for this change was discovered
   (real-life usage, code inspection, or something else)

 - sixth, optionally, implementation considerations and alternate
   approaches that were discarded

If you run "git log", you'll see many good and bad examples to think
over and compare to this goal.  It's hard work to describe one's work
well in terms that other people can understand, but I think it can be
satisfying, and in any event, it's just as necessary as including
comments near confusing code.

Sincerely,
Jonathan

  reply	other threads:[~2012-10-31  1:51 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1351617089-13036-1-git-send-email-felipe.contreras@gmail.com>
     [not found] ` <1351617089-13036-5-git-send-email-felipe.contreras@gmail.com>
2012-10-30 18:12   ` [PATCH v2 4/4] fast-export: make sure refs are updated properly Sverre Rabbelier
2012-10-30 18:47     ` Felipe Contreras
2012-10-30 21:17       ` Sverre Rabbelier
2012-10-30 21:35         ` Felipe Contreras
2012-10-30 21:38           ` Jonathan Nieder
2012-10-30 21:41             ` Felipe Contreras
2012-10-30 21:59           ` Sverre Rabbelier
2012-10-30 22:18             ` Felipe Contreras
2012-10-30 22:35               ` Sverre Rabbelier
2012-10-30 22:56                 ` Felipe Contreras
     [not found] ` <1351617089-13036-2-git-send-email-felipe.contreras@gmail.com>
2012-10-30 18:55   ` [PATCH v2 1/4] fast-export: trivial cleanup Jonathan Nieder
     [not found] ` <1351617089-13036-3-git-send-email-felipe.contreras@gmail.com>
2012-10-30 18:57   ` [PATCH v2 2/4] fast-export: fix comparisson in tests Jonathan Nieder
     [not found] ` <1351617089-13036-4-git-send-email-felipe.contreras@gmail.com>
2012-10-30 18:59   ` [PATCH v2 3/4] fast-export: don't handle uninteresting refs Jonathan Nieder
2012-10-30 19:17     ` Felipe Contreras
2012-10-30 21:40       ` Felipe Contreras
2012-10-30 21:45         ` Jonathan Nieder
2012-10-30 22:01           ` Felipe Contreras
2012-10-30 22:07             ` Jonathan Nieder
2012-10-30 22:22               ` Felipe Contreras
2012-10-30 23:55                 ` Jonathan Nieder
2012-10-31  1:03                   ` Felipe Contreras
2012-10-31  1:08                     ` Jonathan Nieder
2012-10-31  1:39                       ` Felipe Contreras
2012-10-31  1:51                         ` Jonathan Nieder [this message]
2012-10-31  2:22                           ` Felipe Contreras
2012-10-31  0:57     ` Jonathan Nieder
2012-10-31  1:23       ` Felipe Contreras
2012-10-31  1:35         ` Jonathan Nieder
     [not found]   ` <20121030184755.GC15167@elie.Belkin>
     [not found]     ` <CAMP44s2F3qJeGL4V5KZjFNqKA5hrFQKRxXMrakFA6pTNi1DZ3w@mail.gmail.com>
     [not found]       ` <20121030190126.GJ15167@elie.Belkin>
     [not found]         ` <CAMP44s1W4mwK+cNwBqu2S0=Aw04XX9KBan8w4ghyzqbODdmiLQ@mail.gmail.com>
2012-10-30 19:41           ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121031015103.GA15167@elie.Belkin \
    --to=jrnieder@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=srabbelier@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).