All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: submodule-summary
Date: Wed, 14 Oct 2009 23:27:25 +0200	[thread overview]
Message-ID: <4AD6423D.10307@web.de> (raw)
In-Reply-To: <7vfx9lbtpf.fsf_-_@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Dscho condensed his initial patch with the interdiff you mentioned,
>> additionally silenced a compiler warning and activated --first-parent.
>> This follows as patch 1/4. Patches 2/4 to 4/4 contain my two bugfixes
>> and the testcase i copied from submodule summary while adapting it to
>> the changes of the output format.
> 
> I think 2 and 3 should be squashed into the first one.  I do not see any
> good reason for keeping initial "oops that was wrong" etched in stone,
> once the review process has revealed obvious bugs and reasonable fixes
> have been given to them.  If the original author re-spun a v2 patch, that
> is the normal thing that happens.

Right, will do.


> I am not happy with the option name --submodule-summary, by the way.
> Naming this option --submodule-summary shows the confusion between this
> series being the _latest_ great invention and this series being the _last_
> great invention.  I'd freely grant the former but would like to avoid the
> latter.
> 
> I have this nagging suspicion that we should leave the door open for later
> addition of --submodule=full that actually gives the patch text for the
> entire aggregated tree, perhaps recursively.  People may want to add even
> more other useful modes that we do not think of right now. It would be
> better to name this --submodule=shortlog or something.
> 
> If users like the shortlog mode (or the full mode) very much, perhaps the
> current default output, which shows the differences between two commit
> object names, can become a --submodule=summary (or --submodule=twoline)
> mode later, and the shortlog mode could become the default.

Good point. (Personally i like the options --submodule=shortlog and
--submodule=twoline. Because IMHO --submodule=summary could make
people expect similar output to git submodule summary, no?).

Thanks for your feedback, will send new patches soon.

  reply	other threads:[~2009-10-14 21:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-12  3:18 What's cooking in git.git (Oct 2009, #02; Sun, 11) Junio C Hamano
2009-10-12  5:14 ` Jeff King
2009-10-12 22:52   ` Junio C Hamano
2009-10-14  4:24     ` Jeff King
2009-10-12  6:23 ` Sverre Rabbelier
2009-10-14 18:29 ` Jens Lehmann
2009-10-14 18:30   ` [PATCH 1/4] Add the --submodule-summary option to the diff option family Jens Lehmann
2009-10-14 18:31   ` [PATCH 2/4] fix indentation depth for git diff --submodule-summary Jens Lehmann
2009-10-14 18:31   ` [PATCH 3/4] fix output for deleted submodules in " Jens Lehmann
2009-10-14 18:32   ` [PATCH 4/4] add tests for " Jens Lehmann
2009-10-14 20:34   ` submodule-summary Junio C Hamano
2009-10-14 21:27     ` Jens Lehmann [this message]
2009-10-14 22:42       ` submodule-summary Junio C Hamano
2009-10-15 10:34         ` submodule-summary Jens Lehmann

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=4AD6423D.10307@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.