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

2015-06-02 Thread Heiko Voigt
On Sat, May 30, 2015 at 03:25:31PM -0500, Robert Dailey wrote:
> On Sat, May 30, 2015 at 2:54 PM, Junio C Hamano  wrote:
> > Robert Dailey  writes:
> >
> >> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano  wrote:
> >>> Robert Dailey  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
> >
> > And adding one-line-per-commit from side branches does break
> > existing functionality.  You seem to be arguing that more
> > information is always good and does not break existing
> > functionality, but summarizing by omitting irrelevant details *is* a
> > feature.  Do you honestly believe that a change to make the full
> > "log -p" output in submodule log be a "backward compatible" change??
> >
> >> > Merge branch "topic1" into "master"
> >> > Merge branch "topic2" into "master"
> >> > Merge branch "origin/develop" into "master"
> >> > Merge branch "topic3" into "master"
> >
> > It is not a real argument; it is something you can fix by naming
> > your branches more sensibly, which would make you a better developer
> > regardless of how submodule-log is shown.
> 
> I just use git, I don't have the deep technical understanding of its
> implementation as you may have. From my perspective I can't think of
> how this breaks backward compatibility, or perhaps your definition of
> backward compatibility does not align with mine.

Yes from your perspective and thats the only thing what Junio is
criticising: Others might have a different perspective and thats why
this change is not backwards compatible in a general sense. For you it
might be but git is a very general tool.

> And please don't over generalize and misconvey what I said. I am not
> saying more information is always good. Did you not read anything I
> wrote?

I do not see where Junio did that. The only thing he was trying to do is
give an example why changing the default is not backwards compatible.

> Also good branch names may help but that doesn't go into detail and
> explain features that may have been sitting on a topic branch for
> weeks. That's not a practical solution to the problem. Also branch
> names do not determine or influence the skill and quality of a
> developer, as you seem to imply.

Many people use the feature branch name as a kind of headline for the
topic. So in that sense it explains or at least gives a hint what that
feature was about. E.g. if you have the roadmap of a library you are
using in mind and want to know whether the update you are about to
commit contains a certain feature you were waiting for, that headline
can be enough.

You are right: Good commit messages and general naming does
functionality wise not determine your development skills in the short
term. On the other hand: In my experience good naming skills usually
align with good development skills. Long term it also means that your
code (and history) is easier to understand once others want to read your
code and build on it.

> I'll do us both a favor and end the discussion here, as I do not feel
> you are being very patient or welcoming in the discussion. I sense
> frustration on your side.

This is a typical situation which can happen in textual communication.
You read something which is not there. A thing that happened to me as
well when I submitted my first patch on a mailing list. But when I read
the conversation years later I realized it was not really that offending
as I experienced it. A general rule: People automatically appreciate
what you do when they criticise to you. Criticism means that you have
done something good (and worth criticising) but they want to make it
better. I think viewing it that way helps to not take arguments
personally.

>  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.
> >>
> >> 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..."
> >
> > I am not

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

2015-05-30 Thread Robert Dailey
On Sat, May 30, 2015 at 2:54 PM, Junio C Hamano  wrote:
> Robert Dailey  writes:
>
>> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano  wrote:
>>> Robert Dailey  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
>
> And adding one-line-per-commit from side branches does break
> existing functionality.  You seem to be arguing that more
> information is always good and does not break existing
> functionality, but summarizing by omitting irrelevant details *is* a
> feature.  Do you honestly believe that a change to make the full
> "log -p" output in submodule log be a "backward compatible" change??
>
>> > Merge branch "topic1" into "master"
>> > Merge branch "topic2" into "master"
>> > Merge branch "origin/develop" into "master"
>> > Merge branch "topic3" into "master"
>
> It is not a real argument; it is something you can fix by naming
> your branches more sensibly, which would make you a better developer
> regardless of how submodule-log is shown.

I just use git, I don't have the deep technical understanding of its
implementation as you may have. From my perspective I can't think of
how this breaks backward compatibility, or perhaps your definition of
backward compatibility does not align with mine.

And please don't over generalize and misconvey what I said. I am not
saying more information is always good. Did you not read anything I
wrote?

Also good branch names may help but that doesn't go into detail and
explain features that may have been sitting on a topic branch for
weeks. That's not a practical solution to the problem. Also branch
names do not determine or influence the skill and quality of a
developer, as you seem to imply.

I'll do us both a favor and end the discussion here, as I do not feel
you are being very patient or welcoming in the discussion. I sense
frustration on your side.

 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.
>>
>> 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..."
>
> I am not sure how to phrase that any easier. Sorry.

You mean you don't want to? That's fine, if you don't have the will or
patience to explain then I won't bother caring.

> Assuming that you consider output with and without --first-parent
> does not make much of a difference (i.e. "minor difference in
> behaviour"), instead of dropping --first-parent unconditionally,
> like you seem to be pushing with the argument, we can
> unconditionally keep the --first-parent and do not change anything.
> The end result would not make much of a difference either way, and
> not doing anything is much simpler than dropping --first-parent.
>
> Hopefully you can see how absurd that line of reasoning is.  So do
> not make the same argument to push for changing the behaviour
> unconditionally.
>
> If you think the new behaviour can help _some_ users, then you would
> need the feature and a knob to enable it.

First of all, you keep calling this an argument. Perhaps it is for
you, since you're being absurdly rude with me and impatient with the
discussion. This is a brainstorming session. My suggestions may not
seem rational or make sense, but this is natural since I am ignorant
of the finer details of the application.

You're really just overanalyzing my statements from a nonsensical
perspective. I am talking about not adding more settings to an already
complex set of settings. I am not justifying my feature. I think the
feature is just as justified as everything else. Git is FULL of tons
of little options to cater to niche workflows.

I am not fighting against having another option. In fact, that was my
idea to begin with. I am investigating and trying to discuss all
possible approaches and perspectives.

Your attitude is not very welcoming to those that wish to contribute
to the project. In fact, because of your attitude towards me, I will
kindly see myself out. I do not have 

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

2015-05-30 Thread Junio C Hamano
Robert Dailey  writes:

> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano  wrote:
>> Robert Dailey  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

And adding one-line-per-commit from side branches does break
existing functionality.  You seem to be arguing that more
information is always good and does not break existing
functionality, but summarizing by omitting irrelevant details *is* a
feature.  Do you honestly believe that a change to make the full
"log -p" output in submodule log be a "backward compatible" change??

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

It is not a real argument; it is something you can fix by naming
your branches more sensibly, which would make you a better developer
regardless of how submodule-log is shown.

>>> 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.
>
> 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..."

I am not sure how to phrase that any easier. Sorry.

Assuming that you consider output with and without --first-parent
does not make much of a difference (i.e. "minor difference in
behaviour"), instead of dropping --first-parent unconditionally,
like you seem to be pushing with the argument, we can
unconditionally keep the --first-parent and do not change anything.
The end result would not make much of a difference either way, and
not doing anything is much simpler than dropping --first-parent.

Hopefully you can see how absurd that line of reasoning is.  So do
not make the same argument to push for changing the behaviour
unconditionally.

If you think the new behaviour can help _some_ users, then you would
need the feature and a knob to enable it.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-30 Thread Robert Dailey
On Sat, May 30, 2015 at 2:19 PM, Robert Dailey  wrote:
> On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano  wrote:
>> Robert Dailey  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.
>
>>> 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.

I'm having some difficulty with the tests. What it looks like is that
the test repository is created by calling test-lib.sh at the top. Then
by the time the commands after that are run, it's inside the temp
repository.

At the bottom of t4041, I add this:

test_create_repo sm3 &&
git add sm3 &&
cd sm3 &&
echo > foo.txt &&
git add foo.txt &&
git commit -m 'foo.txt' >/dev/null &&
git checkout -b topic >/dev/null &&
echo > topic.txt &&
git add topic.txt &&
git commit -m 'topic.txt' >/dev/null &&
git checkout master >/dev/null &&
git merge --no-ff --no-edit topic >/dev/null &&
cd .. &&
git diff --submodule=log sm3

But this does not work as I expect. How do I add a submodule and then
simulate creation of branches, commits, and merges, prior to running
code in "text_expect_success"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-30 Thread Robert Dailey
On Sat, May 30, 2015 at 12:04 PM, Junio C Hamano  wrote:
> Robert Dailey  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.

>> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-30 Thread Junio C Hamano
Robert Dailey  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"?

> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-30 Thread Heiko Voigt
On Fri, May 29, 2015 at 09:18:11PM -0500, Robert Dailey wrote:
> So I am working on trying to setup my environment (VM through Virtual Box)
> to do some testing on this. You all have encouraged me to try the mailing
> list review model. So I won't give up yet.

I am not sure you need a VM or Linux environment. Of course it will be
helpful in case your tests do no pass on Linux (which they sometimes do
due to some differences between the OSes). But until we actually run
into that problem I do not see anything wrong developing your change
purely on Windows. Since it seems you are more familiar with that
platform I would even encourage you to do so. That reduces the toolset
friction which you might experience in a new environment. Even if you
run into the problem, that your tests do not pass on Linux, we might be
able to solve that on the list.

Have you seen the github pull request -> mailing list proxy thing[1]? If
that helps you maybe you can test it and use it for your patch
submission. I think nobody will be annoyed if we get some strange emails
on the list during that testing phase since that might help more
contributors to contribute.

> 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. 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.
> 
> Thoughts?

This behavior has been with --first-parent for a long time. Even though
it seems like a minor change in my experience there will be complaints
from people that have got used to it and will now get big differences.
You never know how people use it. Since the extra value (full-log)
(AFAIR) has reached a consensus on the list and it allows us to satisfy
both long log and short log users I would prefer to go that route.

Cheers Heiko

[1] http://article.gmane.org/gmane.comp.version-control.git/269699
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-29 Thread Robert Dailey

On 5/21/2015 7:51 AM, Heiko Voigt wrote:

On Tue, May 19, 2015 at 02:29:55PM -0500, Robert Dailey wrote:

On Tue, May 19, 2015 at 5:44 AM, Heiko Voigt  wrote:

On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:

Unfortunately I find it unintuitive and counter productive to perform
inline patches or do anything on a mailing list. Especially on
Windows, it's a pain to setup git to effectively do this. Also I read
mailing lists through Gmail which does not offer a proper monospace
font view or syntax coloring to effectively review patches and
comments pertaining to them.


Are you sure you are not overestimating the effort it takes to send
patches inline? Once you've got your user agent correctly setup its just
a matter of copy and paste instead of attaching the patch. On Windows I
would probably use Thunderbird which has a section in the format-patch
documentation how to configure it. Compared to the effort you probably
spent on writing your patch isn't this bit of extra effort neglectable?
And your patch is almost done. It just needs some tests and maybe a few
rounds on the mailinglist after that.


Since I am not willing to properly follow your process, I will
withdraw my patch. However it is here if someone else wishes to take
it over. Really wish you guys used github's amazing features but I
understand that Linus has already made his decision in that matter.


It not just Linus decision it is also a matter of many people are used
to this workflow. AFAIR there have been many discussions and tries about
using other tools. Email has many advantages which a webinterface does
not provide. It is simply less effort that one person adjusts to this
workflow instead of changing many peoples working workflow.


I'm sorry I couldn't be more agreeable on the matter. Thanks for the
time you spent reviewing my patch.


If you are really this fixed in your workflow that would be too bad.


How do you send your patches inline? Do you use git send-email? I have
tried that and it is horrible to setup. Do you just copy/paste the
patch inline in your compose window?


For bigger patch series I did use send-email but currently I am back to
just using the compose window from whatever email client I am using. On
Windows that would be Thunderbird. But when possible I am not using
Windows.


It would be much simpler to fork Git, create a branch, make my change,
and initiate a pull request. I can get email notifications on comments
to my PR diff and address them with subsequent pushes to my branch
(which would also automatically update the code review). Turn around
times for collaborating on a change are much quicker via Github pull
requests.


I think that depends more on the collaborators than on the tool. When
you get quick replies the turnaround times with both workflows are
quick.

It would be nice if there was a perfect solution for every project that
everyone could use but unfortunately there is not so we sometimes have
to adjust. But I think its more matter of what you are used to. If you
did not have a github account but email software setup you could
complain about the fact that you need to register a github account, fork
git, setup that fork in your local repository, ... instead of just copy
and paste your change into the compose window and then send it to a
mailinglist.


I am willing to review the typical workflow for contributing via git
on mailing lists but I haven't seen any informative reading material
on this. I just find using command line to email patches and dealing
with other issues not worth the trouble. Lack of syntax highlighting,
lack of monospace font, the fact that I'm basically forced to install
mail client software just to contribute a single git patch.


As already mentioned by Stefan there is Documentation/SubmittingPatches
in the Git repository that describes everything and also has a section
on how to do that with Thunderbird.

I tend to not do much on the commandline on Windows since it basically
sucks there. For sending patches you just need

git format-patch HEAD^

and thats it.

Cheers Heiko



So I am working on trying to setup my environment (VM through Virtual 
Box) to do some testing on this. You all have encouraged me to try the 
mailing list review model. So I won't give up yet.


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. 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.


Thoughts?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-22 Thread Roberto Tyley
On Tuesday, 19 May 2015, Stefan Beller  wrote:
> On Tue, May 19, 2015 at 12:29 PM, Robert Dailey
>  wrote:
> > How do you send your patches inline?
>
> This workflow discussion was a topic at the GitMerge2015 conference,
> and there are essentially 2 groups, those who know how to send email
> and those who complain about it. A solution was agreed on by nearly all
> of the contributors. It would be awesome to have a git-to-email proxy,
> such that you could do a git push  master:refs/for/mailinglist
> and this proxy would convert the push into sending patch series to the
> mailing list. It could even convert the following discussion back into
> comments (on Github?) but as a first step we'd want to try out a one
> way proxy.
>
> Unfortunately nobody stepped up to actually do the work, yet :(

I've replied to this on a separate announcement thread on the Git mailing
list here:

http://thread.gmane.org/gmane.comp.version-control.git/269699

...I've created a new tool called submitGit, which aims to help.

> > I am willing to review the typical workflow for contributing via git
> > on mailing lists but I haven't seen any informative reading material
> > on this. I just find using command line to email patches and dealing
> > with other issues not worth the trouble. Lack of syntax highlighting,
> > lack of monospace font, the fact that I'm basically forced to install
> > mail client software just to contribute a single git patch.

I'd be interested to know what you think!

Roberto
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-21 Thread Heiko Voigt
On Tue, May 19, 2015 at 02:29:55PM -0500, Robert Dailey wrote:
> On Tue, May 19, 2015 at 5:44 AM, Heiko Voigt  wrote:
> > On Mon, May 18, 2015 at 10:06:32AM -0500, Robert Dailey wrote:
> >> Unfortunately I find it unintuitive and counter productive to perform
> >> inline patches or do anything on a mailing list. Especially on
> >> Windows, it's a pain to setup git to effectively do this. Also I read
> >> mailing lists through Gmail which does not offer a proper monospace
> >> font view or syntax coloring to effectively review patches and
> >> comments pertaining to them.
> >
> > Are you sure you are not overestimating the effort it takes to send
> > patches inline? Once you've got your user agent correctly setup its just
> > a matter of copy and paste instead of attaching the patch. On Windows I
> > would probably use Thunderbird which has a section in the format-patch
> > documentation how to configure it. Compared to the effort you probably
> > spent on writing your patch isn't this bit of extra effort neglectable?
> > And your patch is almost done. It just needs some tests and maybe a few
> > rounds on the mailinglist after that.
> >
> >> Since I am not willing to properly follow your process, I will
> >> withdraw my patch. However it is here if someone else wishes to take
> >> it over. Really wish you guys used github's amazing features but I
> >> understand that Linus has already made his decision in that matter.
> >
> > It not just Linus decision it is also a matter of many people are used
> > to this workflow. AFAIR there have been many discussions and tries about
> > using other tools. Email has many advantages which a webinterface does
> > not provide. It is simply less effort that one person adjusts to this
> > workflow instead of changing many peoples working workflow.
> >
> >> I'm sorry I couldn't be more agreeable on the matter. Thanks for the
> >> time you spent reviewing my patch.
> >
> > If you are really this fixed in your workflow that would be too bad.
> 
> How do you send your patches inline? Do you use git send-email? I have
> tried that and it is horrible to setup. Do you just copy/paste the
> patch inline in your compose window?

For bigger patch series I did use send-email but currently I am back to
just using the compose window from whatever email client I am using. On
Windows that would be Thunderbird. But when possible I am not using
Windows.

> It would be much simpler to fork Git, create a branch, make my change,
> and initiate a pull request. I can get email notifications on comments
> to my PR diff and address them with subsequent pushes to my branch
> (which would also automatically update the code review). Turn around
> times for collaborating on a change are much quicker via Github pull
> requests.

I think that depends more on the collaborators than on the tool. When
you get quick replies the turnaround times with both workflows are
quick.

It would be nice if there was a perfect solution for every project that
everyone could use but unfortunately there is not so we sometimes have
to adjust. But I think its more matter of what you are used to. If you
did not have a github account but email software setup you could
complain about the fact that you need to register a github account, fork
git, setup that fork in your local repository, ... instead of just copy
and paste your change into the compose window and then send it to a
mailinglist.

> I am willing to review the typical workflow for contributing via git
> on mailing lists but I haven't seen any informative reading material
> on this. I just find using command line to email patches and dealing
> with other issues not worth the trouble. Lack of syntax highlighting,
> lack of monospace font, the fact that I'm basically forced to install
> mail client software just to contribute a single git patch.

As already mentioned by Stefan there is Documentation/SubmittingPatches
in the Git repository that describes everything and also has a section
on how to do that with Thunderbird.

I tend to not do much on the commandline on Windows since it basically
sucks there. For sending patches you just need

git format-patch HEAD^

and thats it.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-04 Thread Jens Lehmann

Am 04.05.2015 um 17:05 schrieb Robert Dailey:

On Fri, May 1, 2015 at 12:57 PM, Heiko Voigt  wrote:

Hi,

On Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:

I am attempting to diff a submodule modified in my working copy and
the only difference is a merge commit. However, I do not get the
"full" range of commits introduced by the merge commit when I diff it:

$ git diff --submodule=log Core
Submodule Core 8b4ec60..def2f3b:
   > Merge remote-tracking branch 'origin/master-ah3k'

However if I go inside my submodule and run `git log` by hand, I get
more information about the TRUE commits introduced:

$ git log --oneline 8b4ec60..def2f3b
def2f3b Merge remote-tracking branch 'origin/master-ah3k'
015c961 Remove log spam in FontManager
7713ba1 Update third party submodule to latest
10aac78 Merge pull request #9 in FE/core from
feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k
287882f FE-1376 Nedd to remain in check detail screen when selecting
donation after SBI
a5a6bed Do not overwrite the current check# within loop
dfb8547 Adding list of checks to CRspChecks before saving
1be280a FE-1354: Guest logged out in specific multiple check scenario
de06d5a [FE-1348] Fix PATT exit while checks still open

It's almost as if the `git diff --submodule=log` approach is passing
in --first-parent to git log, which would exclude commits in the range
that I'm seeing when I run git log manually.


That is exactly the case. In prepare_submodule_summary() that option is
set before doing the revision walk.


Is this by design? Is there a way to enable the full log history with
`git diff` on a submodule?


This stems from the first implementation for showing submodule diffs in
commit 752c0c24. I guess this was done deliberately to limit the amount
of output you get for a submodule. At the moment this is hardcoded but I
think there is nothing wrong with adding another option to include the
full log.

Cheers Heiko


I will go ahead and work on this feature. Here is what I'd like to see:

1. `git diff --submodule` should have the ability to display full logs
vs current logs (i.e. without --first-parent)


I agree. Just recently I started missing that feature too at $DAYJOB.


2. `git submodule summary` should have an option to display full logs
or "first-parent" logs.


No objection against that. Maybe now is a good time to make `git
submodule summary` use `git diff --submodule` internally to make
them behave the same?


For #1, do you recommend adding a 3rd setting for `diff.submodule`
config? Something like "full-log" or something? Or an entirely new
config?


I'd go with a 3rd setting for diff.submodule (and "full-log" would
have been my first choice too ;-).

> I noticed that in diff.h, the DIFF_OPT flags already consume

31 bits. If this is a 32-bit flag, there is only 1 bit left. If we go
with a 3rd setting for `diff.submodule` I think this might consume the
last bit.


Yup. But I'm not sure we can do anything about it.


We could also make `git diff --submodule` default to the "full log"
type, and if users want only first parent logs in submodule summary,
they'd have to execute `git submodule summary` instead.


Please do not change defaults that people lived fine with for years
lightly. But I won't object changing that on a major version if a
majority of users request that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-05-01 Thread Heiko Voigt
Hi,

On Wed, Apr 29, 2015 at 03:53:11PM -0500, Robert Dailey wrote:
> I am attempting to diff a submodule modified in my working copy and
> the only difference is a merge commit. However, I do not get the
> "full" range of commits introduced by the merge commit when I diff it:
> 
> $ git diff --submodule=log Core
> Submodule Core 8b4ec60..def2f3b:
>   > Merge remote-tracking branch 'origin/master-ah3k'
> 
> However if I go inside my submodule and run `git log` by hand, I get
> more information about the TRUE commits introduced:
> 
> $ git log --oneline 8b4ec60..def2f3b
> def2f3b Merge remote-tracking branch 'origin/master-ah3k'
> 015c961 Remove log spam in FontManager
> 7713ba1 Update third party submodule to latest
> 10aac78 Merge pull request #9 in FE/core from
> feature/FE-1348-selecting-continue-on-zero-balance to master-ah3k
> 287882f FE-1376 Nedd to remain in check detail screen when selecting
> donation after SBI
> a5a6bed Do not overwrite the current check# within loop
> dfb8547 Adding list of checks to CRspChecks before saving
> 1be280a FE-1354: Guest logged out in specific multiple check scenario
> de06d5a [FE-1348] Fix PATT exit while checks still open
> 
> It's almost as if the `git diff --submodule=log` approach is passing
> in --first-parent to git log, which would exclude commits in the range
> that I'm seeing when I run git log manually.

That is exactly the case. In prepare_submodule_summary() that option is
set before doing the revision walk.

> Is this by design? Is there a way to enable the full log history with
> `git diff` on a submodule?

This stems from the first implementation for showing submodule diffs in
commit 752c0c24. I guess this was done deliberately to limit the amount
of output you get for a submodule. At the moment this is hardcoded but I
think there is nothing wrong with adding another option to include the
full log.

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html