Re: Diffing submodule does not yield complete logs for merge commits

From: Robert Dailey <rcdailey.lists@gmail.com>
Date: May 30, 2015, 19:19 UTC
Message-ID: <CAHd499B+icDskcsR7zxPfZ=8Nqwg14eb2h2LBuDx6=fnoO24AQ@mail.gmail.com>
In-Reply-To: <xmqqbnh2ateo.fsf@gitster.dls.corp.google.com>

On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

> Robert Dailey <rcdailey.lists@gmail.com> writes:
>
>> In the meantime I'd like to ask, do we even need to add an option for
>> this? What if we just make `diff.submodule log` not use
>> --first-parent? This seems like a backward compatible change in of
>> itself.
>
> Why?  People have relied on submodule-log not to include all the
> noise coming from individual commits on side branches and instead
> appreciated seeing only the overview by merges of side branch topics
> being listed---why is regressing the system to inconvenience these
> existing users "a backward compatible change"?

Backward compatible in the sense that it does not break existing functionality. For example, removing -D from `git branch` would be a backward breaking change.

Also not everyone has a disciplined merge-commit editing policy like the Linux & Git projects do. In fact (especially in corporate environments), most merge commit messages are useless and auto-generated. It's in fact very common (depending on popular tooling) to not have the ability to edit these messages. Example:

    Merge branch "topic" into "master"

This is the defacto message you get when you use Github, Atlassian Stash, etc. any time you merge a PR. For repositories that only accept changes through pull requests, it is not possible to generate a diff log of submodules that is useful without showing the ancestor commits on all parents. Right now 'log' literally gives you the following for a submodule that has a mainline branch that does not accept direct pushes (i.e. only PRs):

    > Merge branch "topic1" into "master"
    > Merge branch "topic2" into "master"
    > Merge branch "origin/develop" into "master"
    > Merge branch "topic3" into "master"

I would argue that this situation is common -- more common than what you would typically see in a well groomed git repository that does not use PRs or always does rebase+ff-merge.

That is the basis for my concern. No functionality is broken and cators to the common case. But I guess since we're discussing 2 distinct workflows, it makes sense to have 2 options for viewing the submodule logs. Because if 'full-log' were indeed the default, it would cause a lot of noise in the well-disciplined-merge-commit workflow.

>From a high level (to explain my motivation for my simplified and

arguably naive suggestion), I work with a lot of amateur users of Git. They always complain about using Git and how "SVN is better". Yet they do not accept the challenge of learning Git, which is a very complicated tool. Much of git I would argue is not very beginner (even user) friendly (although things are certainly getting better). With such an advanced tool, with such complex configuration and behavior, and given all the friction it causes, I can only do my best to offer seemingly sensible alternatives to adding MORE configuration.

Anyway it's just a thought. Glad to get feedback and I see both sides of the fence on this one.

Show 11 quoted lines

>> And it's simpler to implement. I can't think of a good
>> justification to add more settings to an already hugely complex
>> configuration scheme for such a minor difference in behavior.
>
> Careful, as that argument can cut both ways.  If it is so a minor
> difference in behaviour, perhaps we can do without not just an
> option but a feature to omit --first-parent here.  That would be
> even simpler to implement, as you do not have to do anything.
>
> So, if you think the new behaviour can help _some_ users, then you
> would need the feature and a knob to enable it, I would think.

I don't really understand your contrasted example here. Can you explain:

    "...we can do without not just an option but a feature to omit
--first-parent..."

Again thanks for the feedback.