git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Lea Wiemann <lewiemann@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	John Hawley <warthog19@eaglescrag.net>
Subject: Re: Development strategy
Date: Mon, 02 Jun 2008 16:35:35 -0700	[thread overview]
Message-ID: <7vr6bf5s1k.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <48441715.4010507@gmail.com> (Lea Wiemann's message of "Mon, 02 Jun 2008 17:51:49 +0200")

Lea Wiemann <lewiemann@gmail.com> writes:

> ! 2f15087248 perl/Git.pm: add get_hash method
>   abd799435c gitweb: use Git.pm, and use its get_hash method
> ! 5e53e2e67e t/test-lib.sh: add test_external
> ! c5c97f896c Git.pm: fix documentation of hash_object
> ! 8db2d73809 test suite for Git.pm
>   261a54ff5b gitweb: removed unused parse_ref method [not posted yet]
>   e9166526a3 Git.pm: add get_type method [not posted yet]
>
> The patches marked "!" I cannot change without breaking at least one
> subsequent patch, so every time I want to make a change to one of
> those, I also need to change at least one subsequent commit, and
> worse, resubmit it to the mailing list.  (E.g. the the recent rename
> from parse_rev to get_hash necessitated two extra patches to
> accommodate the change.)

If the need to cascade changes is about something minor like naming, do
not be too picky while you are presenting the progress.  I personally
think the method for getting the output from rev-parse should be named
rev_parse and not get_hash, if only not to confuse plumbing users, but do
not scramble and rebase -i many patches only because I said so today.

That will be a waste of your time.  Something small like that, I do not
think anybody would mind if at the very end of the series there is a patch
that modifies all users of get_hash to use rev_parse and rename the
method.  I certainly wouldn't.

If on the other hand the issue is about a design mistake you had in
earlier patches in the series, well, you may have to re-roll the later
ones to adjust to the interface change _if_ you want to eradicate the
design mistake from the history.  But this too depends on the nature of
the mistake.

For example, you may introduce a new sub "foo" in commit #1 to support a
small feature or two you add in commit #2 and #3.  Later, you would want
add yet another new thing that can use something very close to what "foo"
already does, but realize that you need to call "foo" slightly differently
by giving an additional parameter.

You might wish that you had a perfect foresight to have that extra
parameter from the beginning.  But _nobody has perfect foresight_.  I
would not want you to start amending from commit #1 in such a case.  Just
make commit #4 to be "Enhance 'foo' to deal with this new situation", and
that commit will change the implementation of "foo" and add the necessary
extra parameter to the existing calling site you introduced in commit #2
and #3.  Then commit #5 will use that new interface to introduce the new
feature you needed an enhanced "foo" and you go on.  Such a case of "the
helper introduced in earlier round was too limited and it is later found
that it needs enhancement" is not even a mistake you would want to remove
from the history.  It is just a normal course of code evolution. 

Keep your commits small, logically independent steps.

Do not _artificially_ squash the commits --- it is _much worse_ than
following up with enhancement patches.  I think your current patch
granularity is quite fine (in good/bad sense not in big/small sense) and
makes pleasant read.

      parent reply	other threads:[~2008-06-02 23:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-02 15:51 Development strategy Lea Wiemann
2008-06-02 18:30 ` Sverre Rabbelier
2008-06-02 19:09   ` Jakub Narebski
2008-06-02 19:24     ` Sverre Rabbelier
2008-06-02 19:24     ` Johannes Schindelin
2008-06-02 19:39     ` Stephan Beyer
2008-06-02 20:02       ` Johannes Schindelin
2008-06-02 22:36   ` Lea Wiemann
2008-06-02 23:04     ` Sverre Rabbelier
2008-06-02 23:35 ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=7vr6bf5s1k.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=lewiemann@gmail.com \
    --cc=warthog19@eaglescrag.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).