Re: [PATCH v3 2/4] git-submodule: New subcommand 'summary' (2) - show commit summary

["Ping Yin" <pkufranky@gmail.com>]

On Thu, Mar 6, 2008 at 7:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Ping Yin <pkufranky@gmail.com> writes:
>
>  > Example: commit summary for modified submodules sm1-sm5.
> >       --A-->B-->C (in src:354cd45)
>  >         \
>  >          -->D-->E (in dst:3f751e5)
>
>  The ordering of the commits in the above list is (1) unnatural and (2)
>  does not match what you would see with "log --left-right --topo-order
>  C...E".
>
>  I do not think "it shows the path to move from C to E" justifies it.  If
>  you are showing them as a linear list (which you cannot avoid if you are
>  doing a sequence of one-line description), you cannot represent such a
>  "path" anyway (think "merge").  If an appliance project rebinds kernel/
>  path from a project based on linux-2.4 to linux-2.6 while upgrading, such
>  a "path" may not even exist.
>
Good point to show 'log --left-right --topo-order' is better

>  I think I've already said the above to your initial round.  It is a bit
>  dissapointing to see none of the comments were addressed and makes me
>  wonder if I have wasted my time reviewing them again.

Sorry i lost your comments. Maybe it's a long time ago, i will revisit them.

>
>  It also is unnerving that newly added and deleted submodule results in the
>  full history display.  It would be assuring to be able to see and verify
>  what the top commit is (or was in "deleted" case) while you are commiting,
>  but I do not see a reason to show more than that.
>
>  I do not think "then set the limit to 1" is a valid answer to that
>  concern.  Wanting to see a dozen top commits in each for a modified
>  submodule would be sensible (and 3/4 may implement such a limit) but even
>  then showing dozen top commits for new/deleted at the same time would be
>  just an added noise.  Is it really necessary to show more than the top for
>  new/deleted case?
>
Actually, not make much sense to show many commits in delete or add
case. I think showing 2 (the head and tail one) should make more sense
since the head one would be "Initial add ..." in many cases which
doesn't make much sense for the user.

I will resend my patches after taking considering all of your
suggestion today or tommorow.



-- 
Ping Yin