Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-10 Thread Junio C Hamano
Josh Triplett  writes:

>> But submission is less important than review. And for review it is
>> usually better (except gigantic series) to have patch text for review
>> with the review.
>
> Agreed.  However, submission typically requires more work than review,
> because the patch text must remain applicable.  For review, as long as
> the email client you use to respond doesn't do something horrible like
> *re-wrap* the quoted patch text, the result will work as a review.

Yup.  That is why we say "please send patch inline; when asked to
send it as an attachment, please do so".
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-10 Thread Jakub Narębski
On 10 August 2016 at 02:55, Josh Triplett  wrote:
> On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote:
>> Some of these problems I hope public-inbox (or something like
>> it) can fix and turn the tide towards email, again.
>
> This really seems like the dichotomy that drives people towards central
> services like GitHub or GitLab.  We need an alternative that doesn't
> involve email, or at the very least, doesn't require people to use email
> directly.  Half of the pain in the process comes from coaxing email
> clients that don't treat mail text as sacrosanct to leave it alone and
> not mangle it.  (Some of that would go away if we accepted attachments
> with inline disposition, but not all of it.  All of it would go away if
> the submission process just involved "git push" to an appropriate
> location.)

But submission is less important than review. And for review it is
usually better (except gigantic series) to have patch text for review
with the review. And threading. And (meta)-versioning of series.
And place for proof-of-concept / weather-balon patches...

-- 
Jakub Narebski
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-10 Thread Josh Triplett
On Wed, Aug 10, 2016 at 09:30:01AM +0200, Jakub Narębski wrote:
> On 10 August 2016 at 02:55, Josh Triplett  wrote:
> > On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote:
> >> Some of these problems I hope public-inbox (or something like
> >> it) can fix and turn the tide towards email, again.
> >
> > This really seems like the dichotomy that drives people towards central
> > services like GitHub or GitLab.  We need an alternative that doesn't
> > involve email, or at the very least, doesn't require people to use email
> > directly.  Half of the pain in the process comes from coaxing email
> > clients that don't treat mail text as sacrosanct to leave it alone and
> > not mangle it.  (Some of that would go away if we accepted attachments
> > with inline disposition, but not all of it.  All of it would go away if
> > the submission process just involved "git push" to an appropriate
> > location.)
> 
> But submission is less important than review. And for review it is
> usually better (except gigantic series) to have patch text for review
> with the review.

Agreed.  However, submission typically requires more work than review,
because the patch text must remain applicable.  For review, as long as
the email client you use to respond doesn't do something horrible like
*re-wrap* the quoted patch text, the result will work as a review.

Ideally, I'd love to see 1) a review UI that stores line-by-line reviews
into a common format and can translate those to email, and 2) a
mechanism to translate reviews written by email and quoting into the
review format and store them with the repository.

> And (meta)-versioning of series.

I've got a documented format for that. :)

> And place for proof-of-concept / weather-balon patches...

Same place as all other patches, just with an "RFC" tag on them.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Eric Wong
Josh Triplett  wrote:
> On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote:
> > Some of these problems I hope public-inbox (or something like
> > it) can fix and turn the tide towards email, again.
> 
> This really seems like the dichotomy that drives people towards central
> services like GitHub or GitLab.  We need an alternative that doesn't
> involve email, or at the very least, doesn't require people to use email
> directly.  Half of the pain in the process comes from coaxing email
> clients that don't treat mail text as sacrosanct to leave it alone and
> not mangle it.  (Some of that would go away if we accepted attachments
> with inline disposition, but not all of it.  All of it would go away if
> the submission process just involved "git push" to an appropriate
> location.)

I don't mind patches as attachments and did some work a few
months ago to ensure they're individually downloadable in the
public-inbox WWW interface (along with full mboxrd messages)[1].

Fwiw, attachments are preferred in perl5-porters, and it might
be acceptable on LKML, even.  Not my call, though.

Having a push/pull-only workflow would still require some sort
of messaging system to notify others.  Ideally that message
would have the output of "git request-pull" to ensure people are
on the same page; but I'd prefer patches (either attachments or
inline) continue to be sent anyways in case the server is down
or the reader is offline or on a machine without git.

[1] see Brian's (who is new, here) initial email for diff-highlight:
https://public-inbox.org/git/20160728162712.ga29...@tci.corp.yp.com/
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Josh Triplett
On Tue, Aug 09, 2016 at 06:28:00PM +, Eric Wong wrote:
> Some of these problems I hope public-inbox (or something like
> it) can fix and turn the tide towards email, again.

This really seems like the dichotomy that drives people towards central
services like GitHub or GitLab.  We need an alternative that doesn't
involve email, or at the very least, doesn't require people to use email
directly.  Half of the pain in the process comes from coaxing email
clients that don't treat mail text as sacrosanct to leave it alone and
not mangle it.  (Some of that would go away if we accepted attachments
with inline disposition, but not all of it.  All of it would go away if
the submission process just involved "git push" to an appropriate
location.)

- Josh Triplett
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Josh Triplett
On Tue, Aug 09, 2016 at 06:57:05AM -0400, Jeff King wrote:
> On Tue, Aug 09, 2016 at 10:11:30AM +0200, Michael J Gruber wrote:
> 
> > Maybe two more points of input for the discussion:
> > 
> > off-line capabilities
> > =
> > 
> > The current workflow here seems to work best when you are subscribed to
> > the git-ml and have your own (local, maybe selective) copy of git-ml in
> > your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right
> > into git-am and such directly. I'm not sure how important the "off-line"
> > aspect of that is for some of us, and how that could be replicated on
> > GitHub - while PRs and such may be Git based behind the scenes there
> > seems to be no way to clone that info and work from a local clone.
> > (Don't know if GitLab is more open.)
> 
> You can pull it all out via GitHub's HTTP API, but the question is what
> format you would use to store it locally (and which tools you would then
> use to play with it).
> 
> I haven't really tried this lately, though, so I don't know if there is
> information that the API would be missing.
> 
> I do have a dream of writing a tool that sucks in GitHub PRs to a fake
> email thread, lets me make my responses inline in an editor, and then
> pushes it back up as PR comments (finding the right positions based on
> the quoted context).

You might try https://github.com/joeyh/github-backup
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Junio C Hamano
Jeff King  writes:

> On Tue, Aug 09, 2016 at 10:34:11AM -0700, Junio C Hamano wrote:
>
>> It may be a good UI that is optimized for drive-by contributors.  It
>> is just that it is not very well suited (compared to mailing list
>> discussions) to conduct high-volume exchange of ideas and changes
>> efficiently.
>
> I think that's something to ponder; can we have a workflow where
> drive-by contributors can use something that has a lower learning/setup
> curve, but long-term contributors might opt for something more powerful?
>
> I think SubmitGit is a step in that direction.

Yes, agreed 100% with that.  The author of the tool must be praised
by getting added to the Cc: line in this discussion ;-)

> It does still require
> switching to the mailing list for subsequent conversation, though. It
> would be interesting to see something like SubmitGit that puts its own
> email in the "From", and that processes email replies into PR comments,
> and then subsequent PR comments into emails (i.e., part of my "dream tool"
> from earlier). It's not clear to me whether the result would just end up
> being irritating for both sides to use (because it doesn't _quite_
> conform to the norms of each format). But it would be fun to find out.

Perhaps.  I do not know if I like that second and subsequent steps
for SubmitGit, but its first step as currently deployed I am very
happy with.

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 11:50:51AM -0700, Stefan Beller wrote:

> > Could you share your mutt set up pleaaase? I've been wanting this for
> > a long time, but never used mutt long enough to bother with a proper
> > setup like this (I blame gmail).
> 
> 
> That is my complaint^H^H^H^H position, too.
> I always wanted to switch to a more powerful
> setup than git-send-email for sending /gmail for reading,
> but I could not convince myself the steep learning/setup curve
> is worth it eventually as it is "not broken enough" to do the change
> right now.

I think I may have shared it before, but here is the script I use to
send emails. It dumps you in mutt, and then I have:

  macro index,pager b ":set edit_headers=yes:set 
edit_headers=no"

to send the message ("b" is for "bounce", which I think may be another
Pine-ism).

-- >8 --
#!/bin/sh

upstream_branch() {
  current=`git symbolic-ref HEAD`
  upstream=`git for-each-ref --format='%(upstream)' "$current"`
  if test -n "$upstream"; then
echo $upstream
  else
echo origin
  fi
}

get_reply_headers() {
  perl -ne '
if (defined $opt) {
  if (/^\s+(.*)/) {
$val .= " $1";
next;
  }
  print "--$opt=", quotemeta($val), " ";
  $opt = $val = undef;
}
if (/^(cc|to):\s*(.*)/i) {
  $opt = lc($1);
  $val = $2;
}
elsif (/^message-id:\s*(.*)/i) {
  $opt = "in-reply-to";
  $val = $1;
}
elsif (/^subject:\s*\[PATCH v(\d+)/i) {
  print "-v$1 ";
}
elsif (/^$/) {
  last;
}
  '
}

has_nonoption=
for i in "$@"; do
  case "$i" in
-[0-9]) has_nonoption=yes ;;
-*) ;;
 *) has_nonoption=yes
  esac
done

git rev-parse || exit 1

: ${REPLY:=$HOME/patch}
test -e "$REPLY" && eval "set -- `get_reply_headers <\"$REPLY\"` \"\$@\""
test "$has_nonoption" = "yes" || set -- "$@" `upstream_branch`

git format-patch -s --stdout --from "$@" >.mbox
if test -t 1; then
  mutt -e 'set sort=mailbox-order' -f .mbox
else
  perl -lne '
if (/^Subject: (.*)/) {
  $subject = $1;
}
elsif ($subject && /^\s+(.*)/) {
  $subject .= " $1";
}
elsif ($subject) {
  print $subject;
  $subject = undef;
}
  ' .mbox |
  sed -e 's/\[PATCH /[/' \
  -e 's/]/]:/' \
  -e 's/^/  /'
fi
rm -f .mbox
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 08:43:59PM +0200, Duy Nguyen wrote:

> On Tue, Aug 9, 2016 at 1:37 PM, Jeff King  wrote:
> >That's (relatively) easy for me to script via mutt (grab
> >these patches, apply them).
> 
> Could you share your mutt set up pleaaase? I've been wanting this for
> a long time, but never used mutt long enough to bother with a proper
> setup like this (I blame gmail).

It's actually pretty simple. The relevant config from my .muttrc is:

   macro pager,index D 'rm -f $HOME/patch'
   macro pager,index A '~/patch'

I use "~/patch" as a rendezvous point, and then "git am ~/patch" from my
other terminal. That avoids mutt having to know which repo to apply to,
and keeps the "am" process in its own terminal (which is handy if it
runs into conflicts, for example).

So generally I would "D" to clear out the contents of ~/patch, and then
"A" whichever patches I want to apply. I often use mutt's aggregate
selection for that. My bindings are:

  bind index \; tag-pattern
  bind index a tag-prefix

which I think come from pine (which I used for many years before
switching to mutt probably 15 years ago). I don't recall the default
keybindings.

Anyway, you can either tag using a pattern (with ";"), or tag mails
individually (using "t", the default), and then "a-A" to apply the "A"
to all of them (if you are in the habit of tagging all of them and then
doing "A" in one swoop, you could also get rid of the separate "D"
command and just make "A" imply it).

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Stefan Beller
On Tue, Aug 9, 2016 at 11:43 AM, Duy Nguyen  wrote:
> On Tue, Aug 9, 2016 at 1:37 PM, Jeff King  wrote:
>>That's (relatively) easy for me to script via mutt (grab
>>these patches, apply them).
>
> Could you share your mutt set up pleaaase? I've been wanting this for
> a long time, but never used mutt long enough to bother with a proper
> setup like this (I blame gmail).


That is my complaint^H^H^H^H position, too.
I always wanted to switch to a more powerful
setup than git-send-email for sending /gmail for reading,
but I could not convince myself the steep learning/setup curve
is worth it eventually as it is "not broken enough" to do the change
right now.

My experiments with mutts, have left these lines in my
~/.muttrc

> # use shift + A to apply a patch in the working dir
> # macro index A ":unset pipe_decode\n|git am -3\n:set pipe_decode\n"
> # macro pager A ":unset pipe_decode\n|git am -3\n:set pipe_decode\n"
>
> macro index A ":set folder='.'\n:copy-message\n"

(IIRC they were broken for many patches, but I got applying
one patch to work. Which sucks for long email series.)
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Duy Nguyen
On Tue, Aug 9, 2016 at 1:37 PM, Jeff King  wrote:
>That's (relatively) easy for me to script via mutt (grab
>these patches, apply them).

Could you share your mutt set up pleaaase? I've been wanting this for
a long time, but never used mutt long enough to bother with a proper
setup like this (I blame gmail).
-- 
Duy
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Duy Nguyen
On Tue, Aug 9, 2016 at 8:36 PM, Duy Nguyen  wrote:
> On Tue, Aug 9, 2016 at 1:20 AM, Michael Haggerty  wrote:
>> Could you elaborate why you would expect quality and/or quantity of
>> reviews to suffer? I'm really curious, and I'd be happy to pass your
>> feedback along to my colleagues.
>
> Since I have been using github at work for a couple months, I do have
> a few complaints if it will ever become the way of reviewing things in
> git. Some of these may be covered by other people already (I haven't
> read all new mails in this thread yet)

Another super nit thing: use monospace font for commit messages, or at
least have an option for that.
-- 
Duy
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Duy Nguyen
On Tue, Aug 9, 2016 at 1:20 AM, Michael Haggerty  wrote:
> Could you elaborate why you would expect quality and/or quantity of
> reviews to suffer? I'm really curious, and I'd be happy to pass your
> feedback along to my colleagues.

Since I have been using github at work for a couple months, I do have
a few complaints if it will ever become the way of reviewing things in
git. Some of these may be covered by other people already (I haven't
read all new mails in this thread yet)

 - Github PRs seem to encourage seeing changes as a whole, not a
separate commits. Or at least it's not so convenient to view separate
commits (e.g. not easy to go to the next commit)

 - The ability to show all outdated comments, in case I want to search
through them.

 - I have a feeling that commits in PRs are sorted by authordate, not
in topological order. The order of commits being committed is
important sometimes.

 - Not showing leading spaces mixing with TABs, or trailing spaces

 - I would love to have all patches numbered so can refer to them as
1/7, 2/5... instead of just short sha1 (and I think you have the
ability to refer to "1/7 of iteration 2", see next bullet point)

 -I guess you can manage multiple iterations of a topic with one
iteration per PR, then linking them together. It would be nicer to
somehow bake the iteration concept directly to a PR so we can switch
between them, or do interdiff. I know, this is more of a improvement
request than complaint because ML is not really better.

 - Offline support would be very nice. I'm only most of the time, but
sometimes I do work on git stuff offline.

 - We lose the integration with ML, I think. Sometimes the user
reports a bug here, then we reply back with a patch. With github, I
guess we reply back with a PR number, then further discussion may go
there, some discussion may still be on ML.

> * It is easy to summon somebody else into the review conversation by
> @-mentioning them. That person immediately can see the whole history of
> the PR. (This is an improvement on the status quo, where a new entrant
> to a conversation might have to dig through the email trash or an email
> archive to see emails that they overlooked before they were added to the
> CC list.)

On the other hand, we can't just CC anybody anymore because we don't
know if they have a github account (or the account name for that
matter). Or does github allow @-ing email addresses too? We record
people's email address, not github account names.

> * It is possible to search old issues and PRs for additional context.
> (Granted, the history of the project from its ML era would have to be
> searched separately.)

To me searching in email is still better. Maybe I haven't fully
explored github search capabilities
-- 
Duy
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Eric Wong
Michael Haggerty  wrote:
> On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
> > [...]
> > Even requiring every contributor to register with GitHub would be too much
> > of a limitation, I would wager.
> > [...]

> * Discussion of pull requests can be done either
>   * via the website (super easy for beginners but powerful for
> experienced users),
>   * by setting up email notifications for your account and replying to
> those emails, or
>   * via an API.
>   Such discussion is all in markdown, which supports light formatting,
> hyperlinks, and @-mentions.



> Disclaimer: I work for GitHub, but in this email I'm speaking for myself.
> 
> Michael
> 
> [1] I concede that people who refuse on ideological grounds to use
> proprietary software will find this step insurmountable. Perhaps we
> could come up with a workaround for such people.

I'm one of those ideological people and I don't see an
acceptable workaround.  GitHub already has misfeatures designed
to lock people in into centralized messaging:

* pull request feature doesn't work for self-hosted repos
  (this disincentivizes people from running and improving
   git-daemon/git-http-backend/etc...)

* "noreply" email addresses

* @-mentions you wrote about

* custom email notifications

This is a problem with Gitlab, Redmine, etc, too:
they cannot interoperate with each other.

At least for now, large proprietary mail providers like Gmail
still interoperate with whatever Free Software SMTP software I
run.  I dread the day when that is no longer true.

Some of these problems I hope public-inbox (or something like
it) can fix and turn the tide towards email, again.  In
contrast, public-inbox is designed to push decentralization:

* "reply" links are instructions for "git send-email" which
  encourage reply-to-all (this applies to what Jeff said
  about vger going down, I noticed it, too)

* anybody can clone the code + repo, replicate the
  instances, and tweak it to their needs.

* public-inbox.org/git/$MESSAGE_ID/t.atom allows subscriptions
  to Atom feeds without any registration or user-tracking

* Message-IDs are exposed for proper threading and interop

* low-bandwidth, Tor-friendly design to encourage deployments
  even behind NATs and firewalls.

Anyways, my optimistic side might interpret your advocacy as
GitHub already feeling threatened by public-inbox.  I certainly
wouldn't expect it at this stage, but I certainly hope it will
be the case one day :)


Disclaimer: I've always been willing to risk a lifetime of
unemployment for ideology.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 10:34:11AM -0700, Junio C Hamano wrote:

> >The threading in GitHub comments and pull requests is also not great.
> >Each PR or issue is its own thread, but it's totally flat inside.
> >There are no sub-threads to organize discussion, and it's sometimes
> >hard to see what people are replying to.
> 
> It may be a good UI that is optimized for drive-by contributors.  It
> is just that it is not very well suited (compared to mailing list
> discussions) to conduct high-volume exchange of ideas and changes
> efficiently.

I think that's something to ponder; can we have a workflow where
drive-by contributors can use something that has a lower learning/setup
curve, but long-term contributors might opt for something more powerful?

I think SubmitGit is a step in that direction. It does still require
switching to the mailing list for subsequent conversation, though. It
would be interesting to see something like SubmitGit that puts its own
email in the "From", and that processes email replies into PR comments,
and then subsequent PR comments into emails (i.e., part of my "dream tool"
from earlier). It's not clear to me whether the result would just end up
being irritating for both sides to use (because it doesn't _quite_
conform to the norms of each format). But it would be fun to find out.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Junio C Hamano
Jeff King  writes all what I wanted to say, and a lot
more, so I don't have to say much ;-)

>  - I really like the flow of having conversations next to patches. I can
>look at the index of the mailing list folder and see what people are
>talking about, how big the threads are, etc, at a glance. Moving
>between messages and threads involve single keystrokes.
>
>Similarly, having local storage is _fast_. I think GitHub is fine for
>a web app. But when I'm reading a high-volume mailing list, I really
>want to flip around quickly. If there's even 500ms to get to the next
>message or thread, it feels clunky and slow to me. Obviously I spend
>more than 500ms _reading_ most messages (though for some I see the
>first paragraph and immediately jump away). It's just the latency
>when I've decided I'm done with one thing and want to move to the
>next.

Viewing threads in a threaded mail client to help prioritizing
various topics being discussed is what I value the most and I am
not sure how I can be as efficient with the pull-request page.

>The threading in GitHub comments and pull requests is also not great.
>Each PR or issue is its own thread, but it's totally flat inside.
>There are no sub-threads to organize discussion, and it's sometimes
>hard to see what people are replying to.

It may be a good UI that is optimized for drive-by contributors.  It
is just that it is not very well suited (compared to mailing list
discussions) to conduct high-volume exchange of ideas and changes
efficiently.

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Junio C Hamano
Johannes Schindelin  writes:

> On Mon, 8 Aug 2016, Junio C Hamano wrote:
>
>> Unless a patch is about an area you are super familiar with so that you
>> know what is beyond the context of the patch to be able to judge if the
>> change is good in the context of the file being touched, it is always
>> hard to review from inside a mail reader.
>> 
>> Running "git am" is a good first step to review such a patch, as that
>> lets you view the resulting code with the full power of Git.  As you
>> gain experience on the codebase, you'll be able to spot more problems
>> while in your mail reader.
>
> I am glad that you agree that the requirement to manually transform the
> patches back into Git (where they had been originally to begin with) is
> cumbersome. This is the first time that I see you admit it ;-)

I was about to apologize for writing a statement that can be
misread, but I do not think what I wrote can be misinterpreted, even
if a reader deliberately tries to twist the words s/he reads, to
lead to such a conclusion, so I won't.

I merely said that reviewing a change in an unfamiliar area is
harder (not "cumbersome", but "needs understanding first") with a
patch, and it is easier to see changes in context by applying (which
is an easy, not "cumbersome", process).

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Johannes Schindelin
Hi Michael,

On Tue, 9 Aug 2016, Michael Haggerty wrote:

> On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
> > [...]
> > Even requiring every contributor to register with GitHub would be too
> > much of a limitation, I would wager.
> > [...]
> 
> Is it *really* so insane to consider moving collaboration on the Git
> project to GitHub or some other similar platform?

Speaking for myself, I do prefer GitHub's UI to mail, by a lot. Not only
because it is more focused on the code, but because it integrates so
nicely with Git, which email distinctly does not.

So I personally would not have the least bit of a problem to switch to
GitHub (that's indeed what Git for Windows did, getting substantially more
contributions than we would otherwise have).

And of course I use the email notifications quite a bit. They are really
convenient: I get my updates via my mail program, still, and the
discussion I want to participate in is just one click away.

The reason why I stated that GitHub is out of the question is that I
expected resistance against it. But you are right: I should not have ruled
it out so categorically, it is not at all my call to make.

Ciao,
Dscho
--
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: Thunderbird woes, was: Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Michael J Gruber
Jakub Narębski venit, vidit, dixit 09.08.2016 10:24:
> On 9 August 2016 at 10:11, Michael J Gruber  wrote:
> 
>> My own setup
>> 
>>
>> My usual MUA is Thunderbird because of its integration with calendars
>> and address books. I usually read and post to mailing lists via
>> nntp/gmane, that works best for me for several reasons (e.g. archive
>> available when I need it).
>>
>> For git-ml, I had to learn early on to answer by e-mail to git-ml rather
>> than by nntp-reply because proper nntp-replies somehow didn't meet the
>> expectations of the e-mail users (double copies because of the cc-policy
>> or such, I don't remember).
> 
> I use either KNode or Thunderbird with NNTP/Gmane, and I don't have
> any problems when doing "Reply All" even if I forget to remove nntp-reply.
> You should do reply-all anyway, because not everyone is subscribed,
> and not everyone uses nntp-ml.

There used to be a problem (many years ago), so let's see...

> 
>> I use git sendemail even for small throw-in patches because the git-ml
>> does not allow attachments but wants patches (files) as in-line text,
>> and Thunderbird possibly reformats text (because it's text, you know).
> 
> For some strange reason Thunderbird used as NNTP reader does not
> screw up with whitespace, while Thunderbird used as email client does.
> Al least it did last time I forgot to create new email in its NNTP reader.

You mean, "format fl[ao]wed" is not used when posting via NNTP? That
still wouldn't help with reply all.

> Note that git-format-patch has Thunderbird subsection in the
> "MUA specific hints" section.

I know, but that changes config (esp. wrap+flowed) for all e-mails. I
would be nice if using an external editor would turn off TB's mangling.

>> When I want to try out a proposed patch I have to "save message" and run
>> git-am because patches don't come as file attachments on the git-ml
>> (can't use "save/open attachment"+ git-apply) nor a PR (can't use git
>> fetch nor view in browser).
> 
> Inline are preferred over attachment because it is easier to review
> and comment on online patches. Anyway, it is the same amount of
> steps, and git-am preserves commit message.

I know why we do that, but attachments are there for a reason (stable
transport), and sending attached patches would be much easier for many
MUAs. It just shows that for nowadays usage of e-mail (not withstanding
Junio's historic remarks), using in-line text as a stable transport is
really "very special", to put it mildly.

>>If it's a series, I have to do that 
>> for each
>> invididual patch, which usually means I simply don't (or rely on Junio
>> doing it and fetch his xy/topic branch).
> 
> I think you can save-all on the whole thread...

I'll definitely try, thanks.

Michael

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Johannes Schindelin
Hi Junio,

On Mon, 8 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > I think you both got it wrong. The original citizens were the mail
> > clients that allowed you to communicate with other human beings.
> > ... It is our usage to transport machine-readable content (and not
> > as an attachment!) that is the intruder.
> 
> It is not "its is our usage".
> 
> You are too young to remember or too old to remember the history, or
> you are knowingly distorting it.  The original users of "patch" and
> "diff" expected that e-mail to be a medium to safely exchange
> changes to programs among themselves.

If you are saying that transporting patches via email was the original
purpose of email, then it is not exactly I who is misremembering history.

But that is not what you meant, I believe. You probably wanted to point
out that the Git developers are not the first ones to abuse the medium
known as email that way. And you are correct, of course. And I never
claimed anything else. I just said that the problem is our usage of emails
as a means to transport byte-exact content intended primarily to be
consumed by a program instead of a human. It does not matter whether
others did that before us. It is the problem we face right now, that is
the important part of my message.

And even if it seems as if you are eagerly defending this system, I do not
believe even a microsecond that you think it is a good system. I believe
that you, too, would welcome a better review/contribution system that is
easier to use, more welcoming to new users, less error-prone and less
time-wasting than the current, email-based one, just like you jumped on
Git as a better SCM when it came around, from whatever inadequate system
you came from.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Johannes Schindelin
Hi Junio,

On Mon, 8 Aug 2016, Junio C Hamano wrote:

> Lars Schneider  writes:
> 
> > 4.) Reviewing patches is super hard for me because my email client
> > does not support patch color highlighting and I can't easily expand
> > context or look at the history of code touched by the patch (e.g via
> > git blame). I tried to setup Alpine but I wasn't happy with the
> > interface either. I like patches with a GitHub URL for review but then
> > I need to find the right line in the original email to write a
> > comment.
> 
> Unless a patch is about an area you are super familiar with so that you
> know what is beyond the context of the patch to be able to judge if the
> change is good in the context of the file being touched, it is always
> hard to review from inside a mail reader.
> 
> Running "git am" is a good first step to review such a patch, as that
> lets you view the resulting code with the full power of Git.  As you
> gain experience on the codebase, you'll be able to spot more problems
> while in your mail reader.

I am glad that you agree that the requirement to manually transform the
patches back into Git (where they had been originally to begin with) is
cumbersome. This is the first time that I see you admit it ;-)

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 01:20:05AM +0200, Michael Haggerty wrote:

> > but I
> > do not think it is sane to expect that the same quality and quantity
> > of reviews as we do here can be maintained with that thing.
> 
> Could you elaborate why you would expect quality and/or quantity of
> reviews to suffer? I'm really curious, and I'd be happy to pass your
> feedback along to my colleagues.

Having done a lot of review here on the mailing list, as well as in
GitHub PRs, I vastly prefer the mailing list.

Here's a random list of things that I think I would miss:

 - I really like the flow of having the commit message and diff dumped
   in my editor. I'm very efficient at slicing and dicing text, omitting
   uninteresting quoted bits, etc.

   Web text boxes feel like a straitjacket. I do have a browser plugin
   that opens them in vim. That helps, but it breaks the flow (I make a
   comment, save the file, click "comment", then read to the next place,
   click "+", then start a new vim instance for that comment).  Besides
   the tedium of clicking around, it loses the "unit" size of a single
   email, where I may make many comments, go back and revise earlier
   comments after reading more of the patch, etc.

 - I really like the flow of having conversations next to patches. I can
   look at the index of the mailing list folder and see what people are
   talking about, how big the threads are, etc, at a glance. Moving
   between messages and threads involve single keystrokes.

   Similarly, having local storage is _fast_. I think GitHub is fine for
   a web app. But when I'm reading a high-volume mailing list, I really
   want to flip around quickly. If there's even 500ms to get to the next
   message or thread, it feels clunky and slow to me. Obviously I spend
   more than 500ms _reading_ most messages (though for some I see the
   first paragraph and immediately jump away). It's just the latency
   when I've decided I'm done with one thing and want to move to the
   next.

 - For that matter, GitHub doesn't really have a good tool for random
   conversations. There are issues, which you can vaguely use like a
   thread, but it doesn't quite feel the same.

   I think part of it is that I can view the mailing list both as a
   series of threads _and_ as a stream of messages. So sometimes I mark
   a thread as "read", and then see the next day that there are a ton of
   new messages on it. Maybe those are uninteresting (and it's a single
   keystroke to mark the thread again), but maybe that's a hint that
   there's interesting discussion going on.

   The threading in GitHub comments and pull requests is also not great.
   Each PR or issue is its own thread, but it's totally flat inside.
   There are no sub-threads to organize discussion, and it's sometimes
   hard to see what people are replying to.

 - When I move between a discussion and patches on the list and my local
   git checkout, it's important to do so with minimal fuss. Which means
   I want to use _context_ in my workflow. If I'm reading a thread, I
   want there to be a keystroke for "jump to this thread in my
   checkout". That's (relatively) easy for me to script via mutt (grab
   these patches, apply them). It's a bit harder in the browser (the
   best I've got is to copy-paste the URL to a script that pulls out the
   PR number, then fetches and checks it out).

 - A sort-of feature: the mailing list is actually fairly decentralized,
   because of the "reply-to-all" convention. I don't know if anybody
   else noticed, but vger seemed to be down Friday evening and Saturday
   morning (at least my messages to the list got 400 SMTP codes, and no
   new messages were delivered to me). But I still had some
   conversations going with people, because our messages were mailed
   directly (and the list eventually caught up).

   Now that probably doesn't matter for GitHub, which seems to have
   fairly reasonable uptime. It would matter if we picked a centralized
   tool that didn't.

There are probably more, but I've run out of ranting steam for now. :)

> Here are some factors that I think will *improve* reviews:

I was going to respond point-by-point to a few of these, but I think I
covered most of it above. In short, I agree with many of the benefits
you list. In most cases, I've already reaped those benefits for my own
workflow (e.g., my "git am" workflow is pretty efficient now). But not
everybody has done so, and it's a lot to ask of casual contributors.

> Given that I work for GitHub, I'm uncomfortable doing any more advocacy
> here. If people have concrete questions, I'd be happy to answer them, on
> the list or in private.

Hopefully I provided some counterpoint. ;)

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 10:11:30AM +0200, Michael J Gruber wrote:

> Maybe two more points of input for the discussion:
> 
> off-line capabilities
> =
> 
> The current workflow here seems to work best when you are subscribed to
> the git-ml and have your own (local, maybe selective) copy of git-ml in
> your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right
> into git-am and such directly. I'm not sure how important the "off-line"
> aspect of that is for some of us, and how that could be replicated on
> GitHub - while PRs and such may be Git based behind the scenes there
> seems to be no way to clone that info and work from a local clone.
> (Don't know if GitLab is more open.)

You can pull it all out via GitHub's HTTP API, but the question is what
format you would use to store it locally (and which tools you would then
use to play with it).

I haven't really tried this lately, though, so I don't know if there is
information that the API would be missing.

I do have a dream of writing a tool that sucks in GitHub PRs to a fake
email thread, lets me make my responses inline in an editor, and then
pushes it back up as PR comments (finding the right positions based on
the quoted context).

> For git-ml, I had to learn early on to answer by e-mail to git-ml rather
> than by nntp-reply because proper nntp-replies somehow didn't meet the
> expectations of the e-mail users (double copies because of the cc-policy
> or such, I don't remember).

At least some people's workflows seem to send two copies to the list.
For instance, Jakub's <2bfd9cf5-a9fa-7650-21e9-9ceb9cc34...@gmail.com>
got delivered to me via the list twice. Once directly from gmail with:

  To: Oleg Taranenko ,
  Junio C Hamano 
  Cc: git@vger.kernel.org

and once via gmane with:

  To: git@vger.kernel.org
  Cc: git@vger.kernel.org

It's like this with all of his messages (sorry I can't point to the
duplicates in an archive; they have the same message-id, so public-inbox
treats them as a single unit).

Replying to the second one breaks the usual "cc-everybody" rule. Sending
duplicates means everybody sees it twice (3 times if they're on the cc
list!), and the second copy still has the bogus headers (so people
replying need to pick the right one).

> I use git sendemail even for small throw-in patches because the git-ml
> does not allow attachments but wants patches (files) as in-line text,
> and Thunderbird possibly reformats text (because it's text, you know).

I wonder if this is something we could change. I do not personally have
any problem with attached patches. "git am" knows how to apply them, and
mutt is smart enough to show text/* by default, and to include it in
quoted text on reply. So the output of "git format-patch --attach" works
fine for me. But it may not be as nice in other MUAs, and we have to
care about all of the other reviewers.

> When I want to try out a proposed patch I have to "save message" and run
> git-am because patches don't come as file attachments on the git-ml
> (can't use "save/open attachment"+ git-apply) nor a PR (can't use git
> fetch nor view in browser). If it's a series, I have to do that for each
> invididual patch, which usually means I simply don't (or rely on Junio
> doing it and fetch his xy/topic branch).

So you would like the opposite of my dream tool, I think: something that
takes mailing list conversations and turns them into PRs.

(My real dream is actually to have a bidirectional version of the tool,
so that everybody can use whatever interface they like, and nobody has
to care about somebody else's preferences).

> And more often than not, patches from series do not appear in sequence,
> not threaded on top of the cover letter (in the gmane nntp version of
> git-ml), and it usually happens for the same people again and again,
> which tells me it's a git sendemail config issue and not gmane.

Just a guess, but I suspect this is caused by people who use "rebase -i"
to rearrange patches. When format-patch writes out the patches, it uses
the author date as the "Date" field, which means it may be out of order.
I think send-email will always write out a new, monotonically increasing
date. But I suspect other workflows (e.g., imap-send and then mailing
from a MUA) blindly re-use that date.

> Suggestion
> ==
> 
> Maybe the current gmane woes are a good reason to try out something
> different for a month or two, with copies to the git-ml, and with the
> default being to revert back to git-ml after that and discuss what we've
> learned. As a result we may improve our workflow here, get GitHub to
> improve, and maybe switch or not. Either way we could profit from that.

I think public-inbox is a nice step forward on the reading side (it's a
lot easier to get raw patches out of it, for example). But it doesn't
help much with sending (and sending is a tricky subject; anytime you
promise to send mail on behalf of 

Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jeff King
On Tue, Aug 09, 2016 at 10:17:22AM +0100, Richard Ipsum wrote:

> > In the very unlikely event that github is shut down, how do we get all
> > review comments out of it, assuming that we will use pull requests for
> > review?
> 
> For what it's worth this is exactly why I think it would be worthwhile for git
> to establish a common review format, services like Github/Gitlab could then
> start storing reviews and comments in the git repo rather than in a separate
> sql database.

I doubt that the "rather than" part will ever happen. Git does not make
a very good database, and certainly not when you want to do things that
cut across repositories (like, say, efficiently get all review comments
made by one user).

It would be nice to have a common interchange format, though. In theory
that could feed into (and out of) a more efficient representation on the
backend of the site. It doesn't _have_ to be git-based, but it would be
nice if it was.

Somebody asked elsewhere "what happens if GitHub goes away?". And the
answer is that you can already get all of that data out in a
programmatic way, via the API. But since there's no common interchange
format, you'd be stuck writing a conversion to whatever format your new
destination uses.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Michael Haggerty
On 08/09/2016 06:22 AM, Duy Nguyen wrote:
> On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggerty  
> wrote:
>> On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
>>> [...]
>>> Even requiring every contributor to register with GitHub would be too much
>>> of a limitation, I would wager.
>>> [...]
>>
>> Is it *really* so insane to consider moving collaboration on the Git
>> project to GitHub or some other similar platform?
> 
> In the very unlikely event that github is shut down, how do we get all
> review comments out of it, assuming that we will use pull requests for
> review?

I don't have any experience with these tools, but a quick search turns
up the following possibilities (among others):

* github-backup (by Joey Hess): https://github.com/joeyh/github-backup
* python-github-backup: https://github.com/josegonzalez/python-github-backup
* BackHub (commercial service): https://backhub.co/
* Import GitHub project into GitLab:
http://docs.gitlab.com/ce/workflow/importing/import_projects_from_github.html

Michael

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Richard Ipsum
On Tue, Aug 09, 2016 at 06:22:21AM +0200, Duy Nguyen wrote:
> On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggerty  
> wrote:
> > On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
> >> [...]
> >> Even requiring every contributor to register with GitHub would be too much
> >> of a limitation, I would wager.
> >> [...]
> >
> > Is it *really* so insane to consider moving collaboration on the Git
> > project to GitHub or some other similar platform?
> 
> In the very unlikely event that github is shut down, how do we get all
> review comments out of it, assuming that we will use pull requests for
> review?

For what it's worth this is exactly why I think it would be worthwhile for git
to establish a common review format, services like Github/Gitlab could then
start storing reviews and comments in the git repo rather than in a separate
sql database.

Gerrit is already doing this with notedb, which literally gives you a
git log of a review. Admittedly with Gerrit the change metadata
sits in a separate git repo, still,
this is much better than the current situation with
Github and Gitlab in my opinion.

I apologise once again if my comments here are somewhat unrelated,
but I feel there is at least some overlap, since the existence of a
common review format for git could potentially make Github/Gitlab a more
attractive option, if Github/Gitlab chose to adopt such a format.

Really I think that reviews shouldn't be stored on mailing lists,
and they shouldn't be stored in sql databases,
they should be stored in git.
--
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


Thunderbird woes, was: Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Jakub Narębski
On 9 August 2016 at 10:11, Michael J Gruber  wrote:

> My own setup
> 
>
> My usual MUA is Thunderbird because of its integration with calendars
> and address books. I usually read and post to mailing lists via
> nntp/gmane, that works best for me for several reasons (e.g. archive
> available when I need it).
>
> For git-ml, I had to learn early on to answer by e-mail to git-ml rather
> than by nntp-reply because proper nntp-replies somehow didn't meet the
> expectations of the e-mail users (double copies because of the cc-policy
> or such, I don't remember).

I use either KNode or Thunderbird with NNTP/Gmane, and I don't have
any problems when doing "Reply All" even if I forget to remove nntp-reply.
You should do reply-all anyway, because not everyone is subscribed,
and not everyone uses nntp-ml.

> I use git sendemail even for small throw-in patches because the git-ml
> does not allow attachments but wants patches (files) as in-line text,
> and Thunderbird possibly reformats text (because it's text, you know).

For some strange reason Thunderbird used as NNTP reader does not
screw up with whitespace, while Thunderbird used as email client does.
Al least it did last time I forgot to create new email in its NNTP reader.

Note that git-format-patch has Thunderbird subsection in the
"MUA specific hints" section.

> When I want to try out a proposed patch I have to "save message" and run
> git-am because patches don't come as file attachments on the git-ml
> (can't use "save/open attachment"+ git-apply) nor a PR (can't use git
> fetch nor view in browser).

Inline are preferred over attachment because it is easier to review
and comment on online patches. Anyway, it is the same amount of
steps, and git-am preserves commit message.

>If it's a series, I have to do that 
> for each
> invididual patch, which usually means I simply don't (or rely on Junio
> doing it and fetch his xy/topic branch).

I think you can save-all on the whole thread...

Best,
-- 
Jakub Narębski
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-09 Thread Michael J Gruber
Michael Haggerty venit, vidit, dixit 09.08.2016 01:20:
> Given that I work for GitHub, I'm uncomfortable doing any more advocacy
> here. If people have concrete questions, I'd be happy to answer them, on
> the list or in private.

You're doing a great job differentiating between your roles as a member
of the Git devel community and as a GitHub employee, so please keep the
discussion here.

Maybe two more points of input for the discussion:

off-line capabilities
=

The current workflow here seems to work best when you are subscribed to
the git-ml and have your own (local, maybe selective) copy of git-ml in
your (text-based) MUA (mutt, (al)pine, emacs, ...) that can jump right
into git-am and such directly. I'm not sure how important the "off-line"
aspect of that is for some of us, and how that could be replicated on
GitHub - while PRs and such may be Git based behind the scenes there
seems to be no way to clone that info and work from a local clone.
(Don't know if GitLab is more open.)

My own setup


My usual MUA is Thunderbird because of its integration with calendars
and address books. I usually read and post to mailing lists via
nntp/gmane, that works best for me for several reasons (e.g. archive
available when I need it).

For git-ml, I had to learn early on to answer by e-mail to git-ml rather
than by nntp-reply because proper nntp-replies somehow didn't meet the
expectations of the e-mail users (double copies because of the cc-policy
or such, I don't remember).

I use git sendemail even for small throw-in patches because the git-ml
does not allow attachments but wants patches (files) as in-line text,
and Thunderbird possibly reformats text (because it's text, you know).

When I want to try out a proposed patch I have to "save message" and run
git-am because patches don't come as file attachments on the git-ml
(can't use "save/open attachment"+ git-apply) nor a PR (can't use git
fetch nor view in browser). If it's a series, I have to do that for each
invididual patch, which usually means I simply don't (or rely on Junio
doing it and fetch his xy/topic branch).

And more often than not, patches from series do not appear in sequence,
not threaded on top of the cover letter (in the gmane nntp version of
git-ml), and it usually happens for the same people again and again,
which tells me it's a git sendemail config issue and not gmane.

So really, everytime I interact with the git-ml I think about switching
to mutt or such just for git-ml, even though over time I have gotten
used to the number of hoops that I have to jump through if I want to
interact with git-ml.

Suggestion
==

Maybe the current gmane woes are a good reason to try out something
different for a month or two, with copies to the git-ml, and with the
default being to revert back to git-ml after that and discuss what we've
learned. As a result we may improve our workflow here, get GitHub to
improve, and maybe switch or not. Either way we could profit from that.

Michael
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Duy Nguyen
On Tue, Aug 9, 2016 at 12:20 AM, Michael Haggerty  wrote:
> On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
>> [...]
>> Even requiring every contributor to register with GitHub would be too much
>> of a limitation, I would wager.
>> [...]
>
> Is it *really* so insane to consider moving collaboration on the Git
> project to GitHub or some other similar platform?

In the very unlikely event that github is shut down, how do we get all
review comments out of it, assuming that we will use pull requests for
review?
-- 
Duy
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Michael Haggerty
On 08/09/2016 12:36 AM, Junio C Hamano wrote:
> Michael Haggerty  writes:
> 
>> Is it *really* so insane to consider moving collaboration on the Git
>> project to GitHub or some other similar platform?
> 
> I only know how "pull requests" and "issues" discussion in GitHub
> Web interface _currently_ looks like, so if you have even more
> wonderful thing in the works, I _might_ be swayed otherwise,

If I did I couldn't say anyway, so let's assume that current GitHub is
what's on the table [1].

There are a couple of recent code-review improvements that you might
have missed:

* You can now get email updates about your own activity [2]. (Previously
you would only get emails about the activity of other people, which
would leave holes in the email record of the conversation.)

* There is also now better visibility of code review comments regarding
lines that are no longer part of a PR [3].

> but I
> do not think it is sane to expect that the same quality and quantity
> of reviews as we do here can be maintained with that thing.

Could you elaborate why you would expect quality and/or quantity of
reviews to suffer? I'm really curious, and I'd be happy to pass your
feedback along to my colleagues.

Here are some factors that I think will *improve* reviews:

* While you are reviewing patches, you can "zoom out" to see code beyond
the usual diff context. Currently a reviewer who wants more context has
to transition from reading the diff in email to applying the patch and
viewing it in another tool. Then the reviewer has to go back to email to
leave the comment.

* If you want to compile/run/edit/profile the code, you just need to
"git fetch" rather than messing around with "git am". For more involved
suggestions, it is possible to propose a PR against the original PR.

* It is easy to summon somebody else into the review conversation by
@-mentioning them. That person immediately can see the whole history of
the PR. (This is an improvement on the status quo, where a new entrant
to a conversation might have to dig through the email trash or an email
archive to see emails that they overlooked before they were added to the
CC list.)

* It is easy to subscribe/unsubscribe from particular discussions [4].
This makes it easier to follow the discussions you are interested in
without getting swamped with emails about other discussions. You can
unsubscribe from a discussion permanently, or in such a way that a new
@-mention brings you back in.

* It is easy to mention other PRs/commits/issues in a discussion, and
those mentions become clickable links (no jumping back and forth between
email client and web browser). Of course you can also link to arbitrary
URLs (e.g., mailing list archives).

* It is possible to search old issues and PRs for additional context.
(Granted, the history of the project from its ML era would have to be
searched separately.)

Given that I work for GitHub, I'm uncomfortable doing any more advocacy
here. If people have concrete questions, I'd be happy to answer them, on
the list or in private.

Michael

[1] In general, GitHub *does* get better over time, and we would benefit
from any future improvements.
[2] https://github.com/blog/2203-email-updates-about-your-own-activity
[3] https://github.com/blog/2123-more-code-review-tools
[4] https://github.com/blog/2183-improvements-to-notification-emails

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Junio C Hamano
Michael Haggerty  writes:

> Is it *really* so insane to consider moving collaboration on the Git
> project to GitHub or some other similar platform?

I only know how "pull requests" and "issues" discussion in GitHub
Web interface _currently_ looks like, so if you have even more
wonderful thing in the works, I _might_ be swayed otherwise, but I
do not think it is sane to expect that the same quality and quantity
of reviews as we do here can be maintained with that thing.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Michael Haggerty
On 08/04/2016 05:58 PM, Johannes Schindelin wrote:
> [...]
> Even requiring every contributor to register with GitHub would be too much
> of a limitation, I would wager.
> [...]

Is it *really* so insane to consider moving collaboration on the Git
project to GitHub or some other similar platform?

* Many, MANY of the most prominent open-source projects are already
using GitHub. Many potential contributors already know how to use it and
already have accounts. Casual observers (e.g., people who only want to
clone the repo and/or read issues and PRs) don't even need an account.

* Even if you don't already have a GitHub account, it is vastly easier
to create one than to set up our current contribution workflow: figure
out the correct SMTP settings for your email provider, configure
git-send-email, test it and work out the kinks, figure out how to use
git-am (and even then, actually using git-am is a tedious chore for
people who don't use an email client that can run it automatically) [1].
We've seen how difficult our current workflow is by observing GSoC
candidates attempting to send their first patch. What we haven't seen is
the invisible GSoC candidates and other potential contributors who never
even get as far as attempting to send a patch.

* Interactions that involve code are done using Git commands directly,
via exchanging bona fide Git commits. Which means that...

* Commits have unambiguous SHA-1s, which we can use when discussing
them, linking to them, merging them, etc. It will no longer be a matter
of detective work to find out whether a discussion is about v1 or v3 of
a patch series, let alone v3 with some cleanups that were silently added
by Junio.

* Discussion of pull requests can be done either
  * via the website (super easy for beginners but powerful for
experienced users),
  * by setting up email notifications for your account and replying to
those emails, or
  * via an API.
  Such discussion is all in markdown, which supports light formatting,
hyperlinks, and @-mentions.

* GitHub creates permalink URLs for all of the important artifacts:
commits, pull requests, pull request comments, etc. These all can be
referenced easily from any discussion. This web of cross-links
accumulates over time and adds a lot of context to discussions.

* GitHub keeps spam under control.

I admit that if we move to GitHub we would be vulnerable if the company
turns evil or goes bankrupt. But is that really a bigger risk than we
accepted by relying on Gmane (a one-person hobbyist operation) for many
of our historical permalinks, which are now broken? In any case, each of
us has a mirror of the code, and there are utilities for backing up
other GitHub metadata. *If* GitHub becomes evil, there will be a lot of
other open-source projects in the same boat, so I am confident that the
tooling for salvaging such information will quickly become excellent.

Currently we force potential Git contributors to learn an email-based
workflow that is almost unique to this project, rather than steering
them towards a workflow (Git plus, potentially, GitHub) that they
probably already know, or if not is worth learning because the knowledge
will carry across to most other open-source projects, not to mention
their professional careers.

We would want to set down guidelines for how we use GitHub. For example,
we might insist that each version of a patch series (v1, v2, etc.) be
submitted as a fresh pull request with references to the previous
version(s) (which also automatically creates forwards links from the
previous versions to the new version). We might want to set up some
robots to help with repetitive activities, like style review, pinging
the right people, etc.

Junio, I'm very sensitive to the need not to decrease your efficiency.
But isn't there a chance that this could *increase* your efficiency? Is
it worth an experiment?

Is the Git project really such a unique snowflake that we need to use a
workflow (and force it on our contributors) that is different than the
workflows used by most other open-source projects?

Disclaimer: I work for GitHub, but in this email I'm speaking for myself.

Michael

[1] I concede that people who refuse on ideological grounds to use
proprietary software will find this step insurmountable. Perhaps we
could come up with a workaround for such people.

--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Junio C Hamano
Lars Schneider  writes:

> ... then I am always super
> eager to send out a new roll just because I don't want any other reviewer
> to waste time on obviously wrong patches. However, I have the impression
> that frequent re-rolls are frowned upon.

Correct.  A good middle-ground is to just reply with "Yes, thanks
for your suggestion, will fix in the next round", while receiving
review comments.  Good reviewers who value their time will not to
waste their time by responding on a point that has already been
pointed out and acknowledged.

> 4.) Reviewing patches is super hard for me because my email client does not
> support patch color highlighting and I can't easily expand context or 
> look at
> the history of code touched by the patch (e.g via git blame). I tried to 
> setup
> Alpine but I wasn't happy with the interface either. I like patches with 
> a GitHub
> URL for review but then I need to find the right line in the original 
> email to
> write a comment.

Unless a patch is about an area you are super familiar with so that
you know what is beyond the context of the patch to be able to judge
if the change is good in the context of the file being touched, it
is always hard to review from inside a mail reader.

Running "git am" is a good first step to review such a patch, as
that lets you view the resulting code with the full power of Git.
As you gain experience on the codebase, you'll be able to spot more
problems while in your mail reader.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-08 Thread Junio C Hamano
Johannes Schindelin  writes:

> I think you both got it wrong. The original citizens were the mail
> clients that allowed you to communicate with other human beings.
> ... It is our usage to transport machine-readable content (and not
> as an attachment!) that is the intruder.

It is not "its is our usage".

You are too young to remember or too old to remember the history, or
you are knowingly distorting it.  The original users of "patch" and
"diff" expected that e-mail to be a medium to safely exchange
changes to programs among themselves.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-07 Thread Lars Schneider

> On 06 Aug 2016, at 10:58, Johannes Schindelin  
> wrote:
> 
> Hi Stefan,
> 
> just quickly (i.e. addressing only one point, will try to address more at
> a later date) because I want to be mostly offline this weekend:
> 
> On Fri, 5 Aug 2016, Stefan Beller wrote:
> 
>> On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin
>>  wrote:
>>> 
>>> I also hate to break it to you that git-send-email is not going to be
>>> part of any solution.
>> 
>> It's written in perl, so it's not one of the core parts of Git that you
>> mentioned above. I do use it though for my submission process.
> 
> The problem is not Perl, but how fiddly it is to set up. And that you lose
> all the niceties of an email client (e.g. when you want to add a comment
> before the diff stat that is not intended to become a part of the commit
> message).
> 
> But I had an apostrophe last night. I might have been a bit overzealous to
> claim that git-send-email won't be a part of the solution. It cannot be
> a *user-visible* part of any solution, that still holds true.
> 
> So here is the apostrophe: why not implement a bot that listens to the PRs
> on GitHub, and accepts commands such as "@bot please send this
> upstream" via comments on the PR. It would then send the patches to the
> list, from its own email address, on behalf of the contributor.
> 
> Lots of things to kink out, such as: does it need to be moderated? Record
> what was submitted in its own git.git fork? Accept replies and attach them
> to the correct PR? Maybe annotate those replies with the commits whose
> patches were discussed? Maybe send out replies on the PR as emails? Maybe
> try to auto-apply suggested patches? Cc: people who commented on earlier
> iterations of the patch series? Maybe make interaction smarter using an AI
> bot framework?
> 
> If only I had lots of time on my hand, I'd start by prototyping a node.js
> server and hooking it up via webhooks, then show it off so others can
> tinker with it.
> 
> This is not completely unlike submitGit, which was a good first attempt,
> but I never used it because I needed it to do so much more than it already
> did, *and* it complicated everything by requiring users to register with
> an extra step to allow submitGit to send email on their behalf. It also
> made contributing to it harder by choosing some non-standard web app
> framework. Also, I really do not like having to go to a different website
> just to send a GitHub PR to the list.
> 
> Anyway, that was my brain fart for the day.

Great discussion! I would like to share my perspective a someone who is
a (relatively speaking) new Git contributor and who has never interacted
on mailing lists before Git:

1.) "git format-patch" and "git send-email" work great for me. It took some
time to learn how they work but now I have my own "submit.sh" based
on those tools and posting a new series is a piece of cake.

2.) Initially it was hard for me to ensure that my patches don't break build or 
tests on Linux and OS X. Travis CI helps me a lot. I just wished we could
get Windows support, too.

3.) I noticed that I get two types of reviews. The first kind points out things
in my patch that are obviously wrong. The second kind are things that 
require
a discussion. When I get feedback of the first kind, then I am always super
eager to send out a new roll just because I don't want any other reviewer
to waste time on obviously wrong patches. However, I have the impression
that frequent re-rolls are frowned upon. If we would use Git for the patches
instead of email, then I could add "squash" patches to indicate changes in
the current roll that will be squashed in the next roll (I know I could
send squash patches as email, too... but for me that gets confusing 
quickly).

4.) Reviewing patches is super hard for me because my email client does not
support patch color highlighting and I can't easily expand context or look 
at
the history of code touched by the patch (e.g via git blame). I tried to 
setup
Alpine but I wasn't happy with the interface either. I like patches with a 
GitHub
URL for review but then I need to find the right line in the original email 
to
write a comment.

Again, this is just my point of view as a "newbie" and I definitively don't 
expect
the Git community to change their established workflows.

Cheers,
Lars--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-07 Thread Johannes Schindelin
Hi,

On Sat, 6 Aug 2016, Eric Wong wrote:

> Junio C Hamano  wrote:
> > Somebody mentioned "configuring it is hard--why does the user have
> > to know SMTP subtleties", and that may be a valid complaint against
> > the primary part of send-email.  The solution for that is not to
> > discard it with bathwater, but make it just as easy as other MSAs,
> > say, Thunderbird, to configure for an average user who can configure
> > these other MUAs.
> 
> Sadly, the average user does not use an MUA, SMTP or IMAP, anymore.
> It's all webmail or apps using proprietary protocols.
> Embrace, extend, extinguish :<

I think you both got it wrong. The original citizens were the mail clients
that allowed you to communicate with other human beings. Webmail is just a
new generation of the same commodity. It is our usage to transport
machine-readable content (and not as an attachment!) that is the intruder.

It's not making things better if we require users to use a second mail
client for sending out patches, and, oh, it does nothing to help with
reintegrating patches back into Git, were they had been before taking that
perilous and lossy journey through that medium called email.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Johannes Schindelin
Hi Stefan,

just quickly (i.e. addressing only one point, will try to address more at
a later date) because I want to be mostly offline this weekend:

On Fri, 5 Aug 2016, Stefan Beller wrote:

> On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin
>  wrote:
> >
> > I also hate to break it to you that git-send-email is not going to be
> > part of any solution.
> 
> It's written in perl, so it's not one of the core parts of Git that you
> mentioned above. I do use it though for my submission process.

The problem is not Perl, but how fiddly it is to set up. And that you lose
all the niceties of an email client (e.g. when you want to add a comment
before the diff stat that is not intended to become a part of the commit
message).

But I had an apostrophe last night. I might have been a bit overzealous to
claim that git-send-email won't be a part of the solution. It cannot be
a *user-visible* part of any solution, that still holds true.

So here is the apostrophe: why not implement a bot that listens to the PRs
on GitHub, and accepts commands such as "@bot please send this
upstream" via comments on the PR. It would then send the patches to the
list, from its own email address, on behalf of the contributor.

Lots of things to kink out, such as: does it need to be moderated? Record
what was submitted in its own git.git fork? Accept replies and attach them
to the correct PR? Maybe annotate those replies with the commits whose
patches were discussed? Maybe send out replies on the PR as emails? Maybe
try to auto-apply suggested patches? Cc: people who commented on earlier
iterations of the patch series? Maybe make interaction smarter using an AI
bot framework?

If only I had lots of time on my hand, I'd start by prototyping a node.js
server and hooking it up via webhooks, then show it off so others can
tinker with it.

This is not completely unlike submitGit, which was a good first attempt,
but I never used it because I needed it to do so much more than it already
did, *and* it complicated everything by requiring users to register with
an extra step to allow submitGit to send email on their behalf. It also
made contributing to it harder by choosing some non-standard web app
framework. Also, I really do not like having to go to a different website
just to send a GitHub PR to the list.

Anyway, that was my brain fart for the day.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Johannes Schindelin
Hi Philip,

On Fri, 5 Aug 2016, Philip Oakley wrote:

> In the same vein, I always want,with my workflow, to use "fixup!
> ".

Good news for you: this is already supported. See here:

https://github.com/git/git/blob/v2.9.2/git-rebase--interactive.sh#L786-L796

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Johannes Schindelin
Hi Eric,

On Fri, 5 Aug 2016, Eric Wong wrote:

> Johannes Schindelin  wrote:
> > On Thu, 4 Aug 2016, Stefan Beller wrote:
> > > git send-email/format-patch recently learned to include a base commit
> > 
> > You may have noticed that my mail-patch-series.sh-generated code
> > submissions contain that base commit. But they still do not contain the
> > SHA-1s of my local commits corresponding to the patches, and even if they
> > did, the replies with suggested edits would most likely have lost said
> > information.
> > 
> > I also hate to break it to you that git-send-email is not going to be part
> > of any solution.
> 
> I think it ought to be.  Some reasons I like emailing patches are:
> 
> [...]

What I said is that *git-send-email* is not going to be part of any
solution.

Note that I said *git-send-email*, not "emailing patches".

What many people on this list forget is that few email users *ever* touch
their email configuration. Asking them to figure out their SMTP settings
and then to make git-send-email work is, uhm, quite a bit unrealistic.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Eric Wong
Junio C Hamano  wrote:
> Somebody mentioned "configuring it is hard--why does the user have
> to know SMTP subtleties", and that may be a valid complaint against
> the primary part of send-email.  The solution for that is not to
> discard it with bathwater, but make it just as easy as other MSAs,
> say, Thunderbird, to configure for an average user who can configure
> these other MUAs.

Sadly, the average user does not use an MUA, SMTP or IMAP, anymore.
It's all webmail or apps using proprietary protocols.
Embrace, extend, extinguish :<
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Philip Oakley

From: "Johannes Schindelin" 

Hi Philip,

On Fri, 5 Aug 2016, Philip Oakley wrote:


In the same vein, I always want,with my workflow, to use "fixup!
".


Good news for you: this is already supported. See here:

https://github.com/git/git/blob/v2.9.2/git-rebase--interactive.sh#L786-L796



That's odd , I never saw any of that in the documentation...

Blame says it was 68d5d03 (rebase: teach --autosquash to match on sha1 in 
addition to message, 2010-11-04) which was before I discovered Git. Maybe 
another documentation fixup needed ;-)


Mind you I'm not sure about 22c5b13 (rebase -i: handle fixup! fixup! 
in --autosquash, 2013-06-27) which looks to only allow one fixup, but maybe 
I'm misreading. [e.g. recieve multiple fixups from the list, or need extra 
fixups as code of documentation is tested]


The capability is still good to know.

Philip 


--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Richard Ipsum
On Fri, Aug 05, 2016 at 05:24:14PM +0200, Johannes Schindelin wrote:
[snip]
> > 
> > This "unified review storage format" really does seem to be the missing
> > piece.
> 
> FWIW I do not think so. The real trick will be to come up with an
> improvement to the process that lets Junio and Peff continue to work as
> before, because It Works For Them, while at the same time letting other
> people (such as myself) use easy-to-configure tools that add substantial
> convenience.
> 
> Which, to me, means that the missing piece is a clever idea how to
> integrate with the mail-based process, without requiring everybody and her
> dog to switch to a specific mail client.

Fair enough, yes it seems to me that git's own review process
is probably a separate discussion.

As far as review tools such as git-appraise, git-series and git-candidate
are concerned, the review storage format really is the missing piece though,
in my opinion,
at least if we want to live in a world with compatible review tooling.

> 
> > The tool I've been working on for the past year (git-candidate) was
> > initially aimed at contrib[1], and was written in perl solely to satisfy
> > contrib rules. It would have been python otherwise.
> 
> Oh...?
> 
> $ git ls-files contrib/\*.py | wc -l
> 4
> 
> And for that matter:
> 
> $ git ls-files contrib/\*.go | wc -l
> 4

I read this guide[1] before I started, and wanted to be on the safe side.
Maybe that was a mistake... :/

> 
> In fact, there are even PHP scripts:
> 
> $ git ls-files contrib | sed -n 's/.*\.//p' | sort | grep -v '.' |
>   uniq | tr '\n' ' '
> bash c el Git go perl php pl pm py rst sh tcsh txt zsh
> 
> But again, I do not think that it makes sense to focus too much on a
> language, or on a file format, before we came up with a strategy how to
> *not* require everybody to change their current ways.

Fair enough. :)

Thanks,
Richard Ipsum

[1]: https://www.kernel.org/pub/software/scm/git/docs/howto/new-command.html
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-06 Thread Junio C Hamano
Johannes Schindelin  writes:

> The problem is not Perl, but how fiddly it is to set up. And that you lose
> all the niceties of an email client (e.g. when you want to add a comment
> before the diff stat that is not intended to become a part of the commit
> message).

Just this part.  I do not think that is fair to send-email.  You are
blaming its "feature" that allows it to drive format-patch, which I
do not consider is the proper part of the command and to which I
kept saying from the early days of its introduction that I'd never
use it and I think we should discourage its use exactly because it
encourages a bad workflow (i.e. you skip the final proof-reading
before sending out, and you cannot add footnote comments).

Treat it like an MSA just like Thunderbird, just designed to be more
suited to send out patches without corruption, and you will be OK.
You work, commit and write your message with your favourite editor,
do format-patch, reword or add footnote with your favourite editor,
and then send it out.  You can avoid letting other MSAs that may
corrupt whitespaces touch what you will send out if you used
send-email, but that is not mandatory.  As long as your favourite
MSA does not corrupt your message, you can use it.

Somebody mentioned "configuring it is hard--why does the user have
to know SMTP subtleties", and that may be a valid complaint against
the primary part of send-email.  The solution for that is not to
discard it with bathwater, but make it just as easy as other MSAs,
say, Thunderbird, to configure for an average user who can configure
these other MUAs.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Eric Wong
Stefan Beller  wrote:
> On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin
>  wrote:
> > Yet another option would be to have a tool that integrates with the Git
> > repository of the Git mailing list represented by public-inbox.
> 
> So my first reaction to that would be: you could push you patches to
> that public inbox and it is translated to emails sent on your behalf.
> 
> Which is reinventing submitGit just in a more accessible language?

Maybe vger.kernel.org could open its submission (587) port like
Debian does.   Unfortunately, this hurts the ability to Cc:
folks directly and makes vger even more of an SPOF
(I guess submitGit also has this problem)

Fwiw, I have a public-inbox for my miscellaneous patch barf at
s...@80x24.org - https://80x24.org/spew/
http://ou63pmih66umazou.onion/spew/

I will throw up my untested/half-tested patches for various
projects so I can still access it across various NATs and
firewalls.

Using Tor for send-email often works (depending on which
blacklists the exit node is on), but is totally optional
(you'd expose your IP).

==> ~/bin/spew <==
#!/bin/sh
exec torsocks git send-email \
--smtp-domain=80x24.org \
--smtp-debug=1 \
--smtp-server-port=submission \
--smtp-server=80x24.org \
--to ${dest-s...@80x24.org} \
--suppress-cc=all \
"$@"


All: Feel free to use it for any Free Software projects, too;
or better yet, host your own :)
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Josh Triplett
On Mon, Sep 17, 2001 at 10:00:00AM +, Stefan Beller wrote:
> But both send-email as well as mail-patch-series as well as git-series
> are all about the *sending* part. Not about the back and forth part, i.e.
> these don't deal with: "here is a fixup on top". And by that I mean
> receiving mails and applying them. git-am is there as a front-end
> once you obtained the mail, but from what I get, your original problem
> is to get up to date with the latest state, that is either in pu or a proposed
> fixup mail on top of your series?

git-series, at least, is intended to handle the back-and-forth: if you
actually publish the series and not just the final result, someone could
pull the series, make a (non-fast-forwarding) change to that, make a new
series commit, and publish their modified version of your series.  You
could then incorporate that change.  One of the use cases I developed it
for was collaborative development of a patch series.

(That workflow still needs a lot more tool assistance to become fully
usable, not least of which to assist with the process of merging changes
to the series.  Working on 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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Eric Wong
Johannes Schindelin  wrote:
> On Thu, 4 Aug 2016, Stefan Beller wrote:
> > git send-email/format-patch recently learned to include a base commit
> 
> You may have noticed that my mail-patch-series.sh-generated code
> submissions contain that base commit. But they still do not contain the
> SHA-1s of my local commits corresponding to the patches, and even if they
> did, the replies with suggested edits would most likely have lost said
> information.
> 
> I also hate to break it to you that git-send-email is not going to be part
> of any solution.

I think it ought to be.  Some reasons I like emailing patches are:

* there's no taking it back once it's sent

* it's backed up within seconds by thousands of subscribers :)

* doesn't require the reader to have an active connection
  to fetch out-of-band

* doesn't require the reader to be on the same machine capable
  of cloning/building the project

There are times when I've been on a slow machine, or offline
when I wanted to read some patches.

However, I do like including a pull request in cover letters
of a patch series (not necessary for one-offs).



But on a side note, I also find it depressing that SMTP is
uncompressed and TLS compression is (still?) unsafe.  At least I
use ssh tunnels w/ compression for IMAP/SMTP to my own server.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Philip Oakley

From: "Duy Nguyen" 

On Wed, Aug 3, 2016 at 6:07 PM, Johannes Schindelin
 wrote:

It would be a totally different matter, of course, if you used the
branches I publish via my GitHub repository, added fixup! and squash!
commits, published the result to a public repository and then told me to
pull from there, that would make things easier. We could even introduce a
reword! construct, to make the review of the suggested edits of the 
commit

message easier.


On the topic of fixup and squash and everything. Is anyone else
annoyed that the commit title is taken for fixup!, squash!
instructions? After you have added a few of them, "git log --oneline"
becomes useless. All you see is "fixup! A", "fixup! A", "fixup! B",
"fixup! A".

Would it be better to let the user control the title? We still need
the cue "fixup!", "squash!"... at the beginning of the title, but the
original commit reference is appended at the end, like s-o-b lines.


In the same vein, I always want,with my workflow, to use "fixup! 
".


This would be to save trying to retype the title correctly, and simply use 
the abbreviated sha1, which would nicely allow an extra short summaty of 
what the fixup is about.


Philip 


--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Stefan Beller
Hi Johannes,

On Fri, Aug 5, 2016 at 1:20 AM, Johannes Schindelin
 wrote:
> Hi Stefan,
>
> On Thu, 4 Aug 2016, Stefan Beller wrote:
>
>> On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
>>  wrote:
>> >
>> >> If we were to change our workflows drastically, I'd propose to go a
>> >> way[1] similar to notedb in Gerrit, or git-series,
>> >
>> > Gerrit is a huge, non-distributed system. Complex, too. If we change
>> > the patch submission process, we should make things *easier*, not
>> > *harder*. So I think Gerrit is pretty much out of the question.
>>
>> I did not propose to use Gerrit or git-series or git appraise.
>>
>> However whenever I see these projects talking to each other, the talk
>> focuses on a "unified review storage format" pretty often, which would
>> make them compatible with each other.
>
> Unless you have a splendid idea how to integrate that unified format with
> our review process on the mailing list, I think this makes for a fine
> discussion elsewhere. I'd really like to focus on the Git project and its
> patch contribution process in this here thread.
>
>> So then I could choose git-series, while you could go with git appraise
>> (although that is written in go, so maybe too fancy already ;)
>
> I think you misunderstood me in a big way.
>
> New languages are awesome. I play with new toys whenever I find the time
> (and streamlining the contribution process would give me more time for
> that). You are talking to a person who implemented the Householder
> transformation in Postscript, a 6502 assembler in Forth, and a music
> composing system in Emacs Lisp. No language is to fancy for me. "me", as
> in "me, personally".
>
> Now let's think about Git for a moment, and its language choices, and the
> rationale behind them. The majority of the critically important code is
> written in C. Is C a good language? Decent, yes, but of course also
> limiting, Resource leaks are very easy to overlook, and we have a share of
> them. No object orientation, so when we need to "subclass", we have no
> compile time safety. The pre-processor constructs make static analysis
> nigh impossible. Plenty of downsides. So why was it chosen? Developers are
> *familiar* with it, that's why. Similar considerations apply to the use of
> shell scripts, and to Perl, to a certain extent.

Which is changing slowly over time IMHO. The "new generation" of
developers may not have in-depth knowledge of shelll or perl any more,
but rather python or java maybe.

>
> I am not talking about contrib/ of course. That's fair game, it contains
> only non-critical/fringe stuff.
>
> Note that the same rationale goes for choosing to accept patch submissions
> via mail to a list that is not subscribers-only.
>
> When it comes to inviting developers to contribute to your project,
> personal preferences become irrelevant, the deciding factor becomes how
> easy it is to join. Is the language popular, many developers already
> familiar with it? Is the build system readily available? Are the
> maintainers responsive?

I agree. I really do.

I had some discussion at lunch yesterday about different attitudes of open
source projects towards new contributions. An example was the Eclipse
projects that scare off potential new contributors (specially these
fly by single patch submissions) as you need to interact through their
instance of Gerrit after signing a CLA.


Git on the other hand doesn't do
a bad job, e.g. I started with a patch that ought to be a single shot drive
by patch. About 780 others wrote one patch and were never seen again:

$ git shortlog -sne |grep 1 |wc -l
788

So our process is not too bad for the time. However email is as a
communication tool is dying [1] eventually?

Let's examine if we get less one time contributions over time:

for i in $(seq 11 -1 1) ; do
x_1=$(git shortlog -sne --since $i.years --until
$(($i-1)).years |grep 1 |wc -l)
printf "$i\t$x_1\n"
done

11   81
10   94
9175
8148
7139
6118
5108
4149
3109
2106
199

Looking at these numbers the number of one time patch contributions
spiked 9 years ago and declined since then (4 years ago seems to be
an outlier)

And I do not think the decline of one off contributions is because Git as
a project is in decline, but has other reasons. Maybe the project is in better
shape now that there are less one-off worthy contributions? Or the
contribution process is not appealing to a lot of new comers?

[1] I could not find a scientific paper that evaluates communcation
habits of people, but these may give a feel for it:
https://www.quora.com/Why-are-young-people-abandoning-email
https://news.ycombinator.com/item?id=3554466
http://www.twistimage.com/blog/archives/nobody-uses-email-anymore/
http://www.emailisnotdead.com/

>
> I vividly remember my reaction to Darcs, for example. It's written in
> Haskell. I am a 

Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Johannes Schindelin
Hi Junio,

On Thu, 4 Aug 2016, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Like you, I have occasionally been bitten by Junio doing a fixup, and
> > then I end up re-rolling, and lose that fixup.
> 
> ... which usually is caught when I receive the reroll, as I try to apply
> to the same base and compare the result with the previous round.

I find it incredibly nice of you to do those fix-ups, sorry to state that
as clearly only now. It's just that I hate to see your time wasted, in
particular when *I* waste it because I missed the fix-ups in `pu`.

> > But I think such fixups are a calculated risk. Sometimes they save a
> > lot of time, both for the maintainer and the contributor, when they
> > manage to prevent another round-trip of the patch series to the list.
> 
> Yes.

FWIW I am more than just willing to spend a little more time on applying
fix-ups and re-rolling patch series (and dual-publishing them via my
public repository), if it helps lighten your burden.

> > IOW, if the flow is something like:
> >
> >   1. Contributor sends patches. People review.
> >
> >   2. Minor fixups noticed by maintainer, fixed while applying.
> 
> This includes different kinds of things:
> 
> a) Trivially correct fixes given in other people's review.
> 
> b) Minor fixups by the maintainer, to code.
> 
> c) Minor fixups by the maintainer, to proposed log message.
> 
> d) "apply --whitespace=fix" whose result I do not even actively
>keep track of.
> 
> >   3. Only one small fixup needed from review. Contributor sends
> >  squashable patch. Maintainer squashes.
> >
> > then I think that is a net win over sending the whole series again, for
> > the contributor (who does not bother sending), reviewers (who really
> > only need to look at the interdiff, which is what that squash is in the
> > first place), and the maintainer (who can squash just as easily as
> > re-applying the whole series).
> 
> > And that is the flip side. If the flow above does not happen, then step
> > 2 just becomes a pain.
> 
> I think I can
> 
>  * stop taking 2-a).  This is less work for me, but some
>contributors are leaky and can lose obviously good suggestions,
>so I am not sure if that is an overall win for the quality of the
>end product;

If you had a `git commit --reword` command to touch up commit messages,
would that help you, together with the `git commit --fixup` command for
code changes? The branches in `pu` would have your fix-ups as strictly
separate commits on top of the contributed patches, and the branches would
need to be sent through rebase -i before merging to `next`, of course.

The idea would be to not forget your fixups in subsequent iterations, but
simply rebase them on top of the new iteration.

It would still not solve my problem that there is no convenient way to
jump from your commits in `pu` to the corresponding ones in my local
branch. But that is my problem, not yours.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Johannes Schindelin
Hi Richard,

On Fri, 5 Aug 2016, Richard Ipsum wrote:

> On Thu, Aug 04, 2016 at 09:42:18AM -0700, Stefan Beller wrote:
> > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
> >  wrote:
> > >
> > >> If we were to change our workflows drastically, I'd propose to go a
> > >> way[1] similar to notedb in Gerrit, or git-series,
> > >
> > > Gerrit is a huge, non-distributed system. Complex, too. If we change
> > > the patch submission process, we should make things *easier*, not
> > > *harder*. So I think Gerrit is pretty much out of the question.
> > 
> > I did not propose to use Gerrit or git-series or git appraise.
> > 
> > However whenever I see these projects talking to each other, the talk
> > focuses on a "unified review storage format" pretty often, which would
> > make them compatible with each other. So then I could choose
> > git-series, while you could go with git appraise (although that is
> > written in go, so maybe too fancy already ;) Or there could be another
> > new program written in C that follows the "review format".
> 
> This "unified review storage format" really does seem to be the missing
> piece.

FWIW I do not think so. The real trick will be to come up with an
improvement to the process that lets Junio and Peff continue to work as
before, because It Works For Them, while at the same time letting other
people (such as myself) use easy-to-configure tools that add substantial
convenience.

Which, to me, means that the missing piece is a clever idea how to
integrate with the mail-based process, without requiring everybody and her
dog to switch to a specific mail client.

> The tool I've been working on for the past year (git-candidate) was
> initially aimed at contrib[1], and was written in perl solely to satisfy
> contrib rules. It would have been python otherwise.

Oh...?

$ git ls-files contrib/\*.py | wc -l
4

And for that matter:

$ git ls-files contrib/\*.go | wc -l
4

In fact, there are even PHP scripts:

$ git ls-files contrib | sed -n 's/.*\.//p' | sort | grep -v '.' |
uniq | tr '\n' ' '
bash c el Git go perl php pl pm py rst sh tcsh txt zsh

But again, I do not think that it makes sense to focus too much on a
language, or on a file format, before we came up with a strategy how to
*not* require everybody to change their current ways.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Johannes Schindelin
Hi Duy,

On Fri, 5 Aug 2016, Duy Nguyen wrote:

> On the topic of fixup and squash and everything. Is anyone else
> annoyed that the commit title is taken for fixup!, squash!
> instructions?

I do not know about others, but I am not annoyed by those commit titles. I
think they make tons of sense.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Duy Nguyen
On Wed, Aug 3, 2016 at 6:07 PM, Johannes Schindelin
 wrote:
> It would be a totally different matter, of course, if you used the
> branches I publish via my GitHub repository, added fixup! and squash!
> commits, published the result to a public repository and then told me to
> pull from there, that would make things easier. We could even introduce a
> reword! construct, to make the review of the suggested edits of the commit
> message easier.

On the topic of fixup and squash and everything. Is anyone else
annoyed that the commit title is taken for fixup!, squash!
instructions? After you have added a few of them, "git log --oneline"
becomes useless. All you see is "fixup! A", "fixup! A", "fixup! B",
"fixup! A".

Would it be better to let the user control the title? We still need
the cue "fixup!", "squash!"... at the beginning of the title, but the
original commit reference is appended at the end, like s-o-b lines.
-- 
Duy
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Richard Ipsum
On Thu, Aug 04, 2016 at 09:42:18AM -0700, Stefan Beller wrote:
> On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
>  wrote:
> >
> >> If we were to change our workflows drastically, I'd propose to
> >> go a way[1] similar to notedb in Gerrit, or git-series,
> >
> > Gerrit is a huge, non-distributed system. Complex, too. If we change the
> > patch submission process, we should make things *easier*, not *harder*. So
> > I think Gerrit is pretty much out of the question.
> 
> I did not propose to use Gerrit or git-series or git appraise.
> 
> However whenever I see these projects talking to each other, the talk focuses 
> on
> a "unified review storage format" pretty often, which would make them 
> compatible
> with each other. So then I could choose git-series, while you could go with
> git appraise (although that is written in go, so maybe too fancy already ;)
> Or there could be another new program written in C that follows the "review
> format".

This "unified review storage format" really does seem to be the missing
piece. The tool I've been working on for the past year (git-candidate)
was initially aimed at contrib[1], and was written in perl solely
to satisfy contrib rules. It would have been python otherwise.

The feedback from that thread[1], was that while git-candidate itself
seemed interesting it would be unreasonable to bless a particular
tool's format. So it seems to me that even if git-series had been
written in perl rather than rust it could have expected a similar
response to that of git-candidate, possibly.

As Stefan says, if we're able to establish a standard for storing
review data in git then it doesn't really matter what the tools are written in.

For what it's worth my possibly quite shoddy attempt at a library
implementing a possible review format for git[2] is written in perl,
mostly to satisfy contrib requirements.

> >
> > Even requiring every contributor to register with GitHub would be too much
> > of a limitation, I would wager.
> >
> > And when I said I have zero interest in tools that use the "latest and
> > greatest language", I was hinting at git-series. Rust may be a fine and
> > wonderful language. Implementing git-series in Rust, however, immediately
> > limited the potential engagement with developers dramatically.

Ironically contrib's language requirements actually raised the bar for me
because it meant that I had to learn perl.

> >
> > Additionally, I would like to point out that defining a way to store
> > reviews in Git is not necessarily improving the way our code contribution
> > process works. If you want to record the discussions revolving around the
> > code, I think public-inbox already does a pretty good job at that.

I agree, and must apologise if this response has been too off topic,
in any case I hope at least some of it was useful to someone.

Hope this helps,
Richard Ipsum

[1]: http://www.mail-archive.com/git%40vger.kernel.org/msg80972.html
[2]: https://bitbucket.org/richardipsum/perl-notedb
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Eric Wong
Johannes Schindelin  wrote:

Agreed on all above points :>

> On Thu, 4 Aug 2016, Eric Wong wrote:
> > Of course, centralized systems are unacceptable to me;
> > and with that I'll never claim any network service I run
> > will be reliable :)
> 
> Hehehe. I guess that's why the public-inbox is backed by a Git
> repository... BTW is it auto-mirrored anywhere?

Yep, and the code is AGPL so others can always replicate it.

I just have the onions mentioned at the bottom of the HTML pages
running something like:

torsocks git fetch && public-inbox-index && sleep 30

in a loop[1].

http://hjrcffqmbrq6wope.onion/git
http://czquwvybam4bgbro.onion/git

I do encourage anybody who is able to, to run their own mirrors
(off their existing email subscription) so my MX doesn't become
an SPOF, either.  I sorta documented it in my original announcement:

https://public-inbox.org/git/20160710004813.ga20...@dcvr.yhbt.net/
https://public-inbox.org/INSTALL

Feel free to ask me (+ m...@public-inbox.org) for
install/running questions related to public-inbox
(haven't gotten to making it work outside of Debian stable, though).

[1] one of my far-off goals for git be: "git fetch --wait-for-update"
to avoid needless wakeups
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Johannes Schindelin
Hi Eric,

On Thu, 4 Aug 2016, Eric Wong wrote:

> Stefan Beller  wrote:
> > On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
> >  wrote:
> > > I guess I have no really good idea yet, either, how to retain the ease of
> > > access of sending mails to the list, yet somehow keep a strong tie with
> > > the original data stored in Git.
> > 
> > Does it have to be email? Transmitting text could be solved
> > differently as well.
> 
> I've thought a lot about this over the years still think email
> is the least bad.

Not only that: people are *familiar* with it. And they have *access* to
it.

> Anti-spam tools for other messaging systems are far behind,
> proprietary, or non-existent.  bugzilla.kernel.org has been hit
> hard lately and I see plenty of bug-tracker-to-mail spam as a
> result from projects that use web-based bug trackers.

Plus, they are all centralized. Do you want to *require* contibutors to
register with a new service?

> I guess a blockchain (*coin) implementation might work (like
> hashcash is used for email anti-spam), but the ones I've glanced
> at all look like a bigger waste of electricity than email
> filters.

I am not even so much concerned with ecological considerations here. Just
the price of entry would be prohibitive.

> Of course, centralized systems are unacceptable to me;
> and with that I'll never claim any network service I run
> will be reliable :)

Hehehe. I guess that's why the public-inbox is backed by a Git
repository... BTW is it auto-mirrored anywhere?

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Johannes Schindelin
Hi Stefan,

On Thu, 4 Aug 2016, Stefan Beller wrote:

> On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
>  wrote:
> >
> >> If we were to change our workflows drastically, I'd propose to go a
> >> way[1] similar to notedb in Gerrit, or git-series,
> >
> > Gerrit is a huge, non-distributed system. Complex, too. If we change
> > the patch submission process, we should make things *easier*, not
> > *harder*. So I think Gerrit is pretty much out of the question.
> 
> I did not propose to use Gerrit or git-series or git appraise.
> 
> However whenever I see these projects talking to each other, the talk
> focuses on a "unified review storage format" pretty often, which would
> make them compatible with each other.

Unless you have a splendid idea how to integrate that unified format with
our review process on the mailing list, I think this makes for a fine
discussion elsewhere. I'd really like to focus on the Git project and its
patch contribution process in this here thread.

> So then I could choose git-series, while you could go with git appraise
> (although that is written in go, so maybe too fancy already ;)

I think you misunderstood me in a big way.

New languages are awesome. I play with new toys whenever I find the time
(and streamlining the contribution process would give me more time for
that). You are talking to a person who implemented the Householder
transformation in Postscript, a 6502 assembler in Forth, and a music
composing system in Emacs Lisp. No language is to fancy for me. "me", as
in "me, personally".

Now let's think about Git for a moment, and its language choices, and the
rationale behind them. The majority of the critically important code is
written in C. Is C a good language? Decent, yes, but of course also
limiting, Resource leaks are very easy to overlook, and we have a share of
them. No object orientation, so when we need to "subclass", we have no
compile time safety. The pre-processor constructs make static analysis
nigh impossible. Plenty of downsides. So why was it chosen? Developers are
*familiar* with it, that's why. Similar considerations apply to the use of
shell scripts, and to Perl, to a certain extent.

I am not talking about contrib/ of course. That's fair game, it contains
only non-critical/fringe stuff.

Note that the same rationale goes for choosing to accept patch submissions
via mail to a list that is not subscribers-only.

When it comes to inviting developers to contribute to your project,
personal preferences become irrelevant, the deciding factor becomes how
easy it is to join. Is the language popular, many developers already
familiar with it? Is the build system readily available? Are the
maintainers responsive?

I vividly remember my reaction to Darcs, for example. It's written in
Haskell. I am a mathematician originally, so Haskell appeals to me. Did
the choice of language appear to be designed to keep contributors out? To
me, it looked that way.

Other example: submitGit. I really like what its intention. For a while, I
even hoped to move my *own* patch submissions to submitGit. I planned to
help get the kinks out of the code by contributing to it. It is written in
Scala, using a web application and testing framework I have not
encountered elsewhere. I struggled with installing it locally and wrapping
my head around the coding paradigms, for 1.5 days (which was all I could
afford at that time). Then I had to give up. Which made me very sad. I
would not have written my mail-path-series.sh tool if submitGit had been
written in node.js, for example, with which I am familiar enough to jump
right in.

So I hope you understand better now why I find Rust a poor choice for
something like git-series, because it should not waste contributors' time
by insisting that their price of entry is learning a new language they are
unfamiliar with, using a new packaging system, installing a new build
setup. I would find Clojure, Crystal or Swift just as poor a choice. Even
node.js. It is just too much of a "Keep Out" sign for busy developers. And
all the developers worth their salt are busy.

> > Additionally, I would like to point out that defining a way to store
> > reviews in Git is not necessarily improving the way our code
> > contribution process works. If you want to record the discussions
> > revolving around the code, I think public-inbox already does a pretty
> > good job at that.
> 
> Yeah recording is great, but we want to talk about replying and
> modifying a series? So if I see a patch flying by on the mailing list,
> ideally I could attach a "!fixup, signed off by Stefan" thing to that
> patch. (I said "thing" as I do not necessarily mean email here.

Right. I briefly considered suggesting a new tool that would operate on
attachments, integrating tightly with the local git.git checkout. Briefly.
I had to reject this idea because I do not think that requiring new tools
just to contribute to Git would fly well.

> > I guess I have no 

Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-05 Thread Jeff King
On Thu, Aug 04, 2016 at 02:12:22PM -0700, Junio C Hamano wrote:

> >   2. Minor fixups noticed by maintainer, fixed while applying.
> 
> This includes different kinds of things:
> 
> a) Trivially correct fixes given in other people's review.
> 
> b) Minor fixups by the maintainer, to code.
> 
> c) Minor fixups by the maintainer, to proposed log message.
> 
> d) "apply --whitespace=fix" whose result I do not even actively
>keep track of.
>
> [...]
>
> I think I can
> 
>  * stop taking 2-a).  This is less work for me, but some
>contributors are leaky and can lose obviously good suggestions,
>so I am not sure if that is an overall win for the quality of the
>end product;

Actually, I think the 2-a class is what often saves a re-roll. Somebody
points out a typo in a commit message or a comment, and it quite often
gets picked up by you without having another round-trip to the list.

If you want to save work by not doing so, that's fine with me. But this
is the gamble I was talking about. I think it's actually often less work
to do the fixup than to look at another re-roll (especially with the
"leaky contributor" thing where you have to make sure all fixes were
applied). So it's a win if it saves the re-roll, but sometimes you end
up having to look at the re-roll anyway.

>  * do a separate "SQUASH???" commit and send them out for 2-b).
>This is a lot more work for a large series, but about the same
>amount of work (except for "remembering to send them out" part)
>for a small-ish topic.  A contributor needs to realize that I
>deal with _all_ the incoming topics, not just from topics from
>one contributor, and small additional work tends to add up.

I think these are largely the same as 2-a. You are just wearing two
hats, reviewer and maintainer. Which I guess lets you take a shortcut
sometimes (and just fix without mentioning it), but fundamentally the
"gamble" aspect is the same, I think.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Junio C Hamano
Jeff King  writes:

> Like you, I have occasionally been bitten by Junio doing a fixup, and
> then I end up re-rolling, and lose that fixup.

... which usually is caught when I receive the reroll, as I try to
apply to the same base and compare the result with the previous
round.

> But I think such fixups are a calculated risk. Sometimes they save a lot
> of time, both for the maintainer and the contributor, when they manage
> to prevent another round-trip of the patch series to the list.

Yes.

> IOW, if the flow is something like:
>
>   1. Contributor sends patches. People review.
>
>   2. Minor fixups noticed by maintainer, fixed while applying.

This includes different kinds of things:

a) Trivially correct fixes given in other people's review.

b) Minor fixups by the maintainer, to code.

c) Minor fixups by the maintainer, to proposed log message.

d) "apply --whitespace=fix" whose result I do not even actively
   keep track of.

>   3. Only one small fixup needed from review. Contributor sends
>  squashable patch. Maintainer squashes.
>
> then I think that is a net win over sending the whole series again, for
> the contributor (who does not bother sending), reviewers (who really
> only need to look at the interdiff, which is what that squash is in the
> first place), and the maintainer (who can squash just as easily as
> re-applying the whole series).

> And that is the flip side. If the flow above does not happen, then step
> 2 just becomes a pain.

I think I can

 * stop taking 2-a).  This is less work for me, but some
   contributors are leaky and can lose obviously good suggestions,
   so I am not sure if that is an overall win for the quality of the
   end product;

 * do a separate "SQUASH???" commit and send them out for 2-b).
   This is a lot more work for a large series, but about the same
   amount of work (except for "remembering to send them out" part)
   for a small-ish topic.  A contributor needs to realize that I
   deal with _all_ the incoming topics, not just from topics from
   one contributor, and small additional work tends to add up.

to reduce #2.  Essentially, doing these two are about moving more
responsibility of keeping track of good suggestions in the review
discussion to the contributor, but a bad thing is that it does not
mean that the maintainer can stop keeping track of them.  We need a
way to make sure leaky contributors do not repeat the same issue in
their reroll that has already been pointed out and whose solutions
presented in the previous review.  Fetching from 'pu' and compare
has been one way to do so (that is why I publish 'pu' in the first
place, not to "build on", but to "view"), but apparently not many
contributors are comfortable with that idea.

There is no good way to reduce 2-c) and 2-d), but on the other hand,
there is no strong need for a special channel to propagate these
back.  2-c) can be a regular review comment (but again that risks
"the leaky contributor" problem) and 2-d) will happen automatically
when applying the rerolled version.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Eric Wong
Stefan Beller  wrote:
> On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
>  wrote:
> > I guess I have no really good idea yet, either, how to retain the ease of
> > access of sending mails to the list, yet somehow keep a strong tie with
> > the original data stored in Git.
> 
> Does it have to be email? Transmitting text could be solved
> differently as well.

I've thought a lot about this over the years still think email
is the least bad.

Anti-spam tools for other messaging systems are far behind,
proprietary, or non-existent.  bugzilla.kernel.org has been hit
hard lately and I see plenty of bug-tracker-to-mail spam as a
result from projects that use web-based bug trackers.

And email spam filtering isn't even that great
(and I think it needs to be better for IPv6 and .onion adoption
 since much of it is still IPv4-oriented blacklisting).

I guess a blockchain (*coin) implementation might work (like
hashcash is used for email anti-spam), but the ones I've glanced
at all look like a bigger waste of electricity than email
filters.


Of course, centralized systems are unacceptable to me;
and with that I'll never claim any network service I run
will be reliable :)
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Jeff King
On Thu, Aug 04, 2016 at 05:29:52PM +0200, Johannes Schindelin wrote:

> So my idea was to introduce a new --reword= option to `git commit`
> that would commit an empty patch and let the user edit the commit message,
> later replacing the original one with the new one. This is not *quite* as
> nice as I want it, because it makes the changes unobvious. On the other
> hand, I would not find a series of sed commands more obvious, in
> particular because that limits you in the ways of sed. And, you know,
> regexp. I like them, but I know many people cannot really work with them.

I don't have a real opinion on this. I probably wouldn't use it, but I
have no problem with it existing. I think it's somewhat orthogonal to
the idea of _transmitting_ those reword operations to somebody else.

> > That pushes work onto the submitter, but saves work from the reviewers,
> > who can quickly say "something like this..." without having to worry
> > about making a full change, formatting it as a diff, etc.
> > 
> > I do think that's the right time-tradeoff to be making, as we have more
> > submitters than reviewers.
> 
> I agree that it is the right trade-off. TBH I was shocked when I learned
> how much effort Junio puts into applying my patches. I do not want that. I
> want my branch to reflect pretty precisely (modulo sign-off, of course)
> what is going to be integrated into Git's source code.

Like you, I have occasionally been bitten by Junio doing a fixup, and
then I end up re-rolling, and lose that fixup (or have to deal with
porting it forward with awkward tools).

But I think such fixups are a calculated risk. Sometimes they save a lot
of time, both for the maintainer and the contributor, when they manage
to prevent another round-trip of the patch series to the list.

IOW, if the flow is something like:

  1. Contributor sends patches. People review.

  2. Minor fixups noticed by maintainer, fixed while applying.

  3. Only one small fixup needed from review. Contributor sends
 squashable patch. Maintainer squashes.

then I think that is a net win over sending the whole series again, for
the contributor (who does not bother sending), reviewers (who really
only need to look at the interdiff, which is what that squash is in the
first place), and the maintainer (who can squash just as easily as
re-applying the whole series).

It does mean the "final" version of the series is never on the list. It
has to be pieced together from the squash (and sometimes step 2 is not
even mentioned on-list).

So I think it is really a judgement call for step (3) on what is a
"small" fixup, and whether it is easier for everybody to look at the
squash interdiff and say "yep, that's right", versus re-reviewing the
whole series.

> I'd much prefer to resubmit a cleaned-up version, even if it was just the
> commit subjects, and be certain that `pu` and my branch are on the same
> page.
> 
> Instead, Junio puts in a tremendous amount of work, and it does not help
> anybody, because the local branches *still* do not have his fixups, and as
> a consequence subsequent iterations of the patch series will have to be
> fixed up *again*.

And that is the flip side. If the flow above does not happen, then step
2 just becomes a pain.

I don't have a silver bullet or anything. I'm mostly just musing.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Stefan Beller
On Thu, Aug 4, 2016 at 8:58 AM, Johannes Schindelin
 wrote:
>
>> If we were to change our workflows drastically, I'd propose to
>> go a way[1] similar to notedb in Gerrit, or git-series,
>
> Gerrit is a huge, non-distributed system. Complex, too. If we change the
> patch submission process, we should make things *easier*, not *harder*. So
> I think Gerrit is pretty much out of the question.

I did not propose to use Gerrit or git-series or git appraise.

However whenever I see these projects talking to each other, the talk focuses on
a "unified review storage format" pretty often, which would make them compatible
with each other. So then I could choose git-series, while you could go with
git appraise (although that is written in go, so maybe too fancy already ;)
Or there could be another new program written in C that follows the "review
format".


>
> Even requiring every contributor to register with GitHub would be too much
> of a limitation, I would wager.
>
> And when I said I have zero interest in tools that use the "latest and
> greatest language", I was hinting at git-series. Rust may be a fine and
> wonderful language. Implementing git-series in Rust, however, immediately
> limited the potential engagement with developers dramatically.
>
> Additionally, I would like to point out that defining a way to store
> reviews in Git is not necessarily improving the way our code contribution
> process works. If you want to record the discussions revolving around the
> code, I think public-inbox already does a pretty good job at that.

Yeah recording is great, but we want to talk about replying and modifying
a series? So if I see a patch flying by on the mailing list, ideally I could
attach a "!fixup, signed off by Stefan" thing to that patch. (I said "thing"
as I do not necessarily mean email here.

>
> I guess I have no really good idea yet, either, how to retain the ease of
> access of sending mails to the list, yet somehow keep a strong tie with
> the original data stored in Git.

Does it have to be email? Transmitting text could be solved differently as well.

With git push/fetch we can interact with a git remote and pertain the state
(commits, ancestor graph) at a full level even including notes that comment
on commits.

git send-email/format-patch recently learned to include a base commit
(xy/format-patch-base), maybe we need a counter part to git send-email
that downloads a series from your mailbox, such that a local branch
can be transmitted to via

"git send-email --base=origin/master --include-notes --name=sb/new-series"

and completely reconstructed (i.e. the commit sha1s even match) including
notes via:

git fetch-email --name=sb/new-series

That way would ensure we have a "simple" way to transmit patches back and forth
and adding potential fixups.


You wrote:
> In short, I agree that our patch submission process is a saber tooth tiger
> that still reflects pre-Git times. While we use Git's tools, the workflow
> really tries to cut out Git as much as possible, in favor of pure mails
> with non-corrupted, non-HTML patches in them, a charmingly anachronistic
> requirement until you try to use state-of-the-art mail clients to send
> them.

And there are two ways out:
* either we teach git how to deal with emails (completely, i.e.
sending+receiving)
* or we change the development model (e.g. no emails any more)

There is no golden third way IMHO.

Thanks,
Stefan
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Johannes Schindelin
Hi Stefan,

On Wed, 3 Aug 2016, Stefan Beller wrote:

> On Wed, Aug 3, 2016 at 9:07 AM, Johannes Schindelin
>  wrote:
> >
> > On Wed, 3 Aug 2016, Junio C Hamano wrote:
> >
> >> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
> >>  wrote:
> >> >
> >> > I disagree, however, with the suggestion to sift through your `pu`
> >> > branch and to somehow replace local branches with the commits found
> >> > there.
> >>
> >> To be more in line with the "e-mailed patch" workflow, I think what I
> >> should do is to send the version I queued with fixups back to the
> >> list as follow-up.  Just like reviewers review, the maintainer
> >> reviews and queues, the original author should be able to work in the
> >> same workflow, i.e. reading and applying an improved version of the
> >> patch from her mailbox.
> >
> > You seem to assume that it isn't cumbersome for people like me to
> > extract patches out of mails and to replace existing commits using
> > those patches.
> >
> > So it probably comes as a huge surprise to you to learn that this *is*
> > cumbersome for me.
> 
> It is also cumbersome for me, because I never had the need to setup a
> proper mail client that has the strength to apply patches. The need was
> not there as I tend to apply only rarely patches by email, so I can go
> the painful way each time.

The reason is clear, too. Mail clients serve humans. That is their
purpose. Humans do not care all that much whether the text was preserved
exactly as the sender wrote it, except rich text (read: HTML), of course.

> > I got too used to the ease of git push, git pull with or without
> > --rebase, and many other Git commands. Having to transmogrify code
> > changes from commits in Git into a completely different universe:
> > plain text patches in my mailbox, and back, losing all kinds of data
> > in the process, is just not at all that easy. And it costs a lot of
> > time.
> >
> > In short: if you start "submitting patches" back to me via mail, it
> > does not help me. It makes things harder for me. In particular when
> > you add your sign-off to every patch and I have to strip it.
> 
> You don't have to strip the sign off, as it shows the flow of the patch,
> e.g.
> 
> Signed-off-by: Johannes Schindelin 
> Signed-off-by: Junio C Hamano 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Johannes Schindelin 
> Signed-off-by: Junio C Hamano 
> 
> may indicate you proposed a patch, Junio picked it up (and fixed a typo
> optionally), I obtained the patch (via mail, via Git?) improved it, you
> improved it further and then Junio took it and merged it upstream.

Recently, I got yelled at because I took one of Junio's patches, made a
couple of changes, and *added* my sign-off.

Before that incident, I agreed with you that it may make for a nice record
of the back-and-forth that eventually resulted in the patch in question.
Now, I am not so sure anymore.

> > If you change your workflow, I would humbly request that you do it in
> > a way that makes things easier on both sides, not harder.
> 
> When attending the Git Merge conference in May, gregkh said roughtly:
> "We deliberately waste developers time, because it is not scarce.
> Maintainers time is scarce however " and it stuck with me. (and I am a
> developer, not a maintainer ;( so at least the kernel community deems it
> ok to waste my time).

Yeah. It was not the only thing I disagreed with in his talk. To be a
little bit blunt (by my standards, not by LKML's standards, that is): the
Linux kernel mailing list is not necessarily anything I would want to use
as a role model.

I agree that maintainers' time is scarce.

I am one.

So of course I agree with that statement. What I disagree with is that it
is okay to *waste* contributors' time. That's just inconsiderate. And I
say that also because I am a contributor *in addition* to being a
maintainer.

As a consequence, I commend Greg for recognizing that the patch submission
process must be light on the maintainer. And I would have commended him
even further if he had realized that proper tooling should waste nothing,
and no one's time.

> While that is true for the kernel community, I guess it is also true for
> the Git community, unless Junio (and the community) want to appoint a
> bunch of maintainer lieutenants, such that they outnumber the number of
> developers, e.g. divided by areas of the code: a refs backend
> maintainer, a submodule maintainer, ...  or rather by area of usage: a
> porcelain UI maintainer, a git-on-server maintainer.

As I mentioned earlier, I do not care much about following LKML's example.

What I see on this here list is that many a potential contributor is
scared away, that we waste precious time (also the maintainer's) pointing
out in what way certain contributions do not follow the guide lines, and
that even 

Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-04 Thread Johannes Schindelin
Hi Peff,

On Wed, 3 Aug 2016, Jeff King wrote:

> On Wed, Aug 03, 2016 at 09:53:18AM -0700, Junio C Hamano wrote:
> 
> > > Leaving aside Dscho's questions of whether pulling patches from email is
> > > convenient for most submitters (it certainly is for me, but I recognize
> > > that it is not for many), I would much rather see incremental fixup
> > > patches from you than whole "here's what I queued" responses.
> > 
> > Ah, yes, I misspoke.  It should be either an incremental diff or
> > in-line comment to spell out what got changed as a response to the
> > patch.
> > 
> > I find myself fixing the title the most often, which is part of the
> > "log message" you pointed out that would not convey well with the
> > "incremental diff" approach.
> 
> I mentioned a micro-format elsewhere in my message. And it certainly is
> nice to have something that can be applied in an automatic way.

Indeed. This is what I meant by my (succinct to the point of being
intelligible, admittedly) reword! suggestion.

Let's clarify this idea.

I find myself using fixup! and squash! commits a lot. Actually, let me
pull out the Linux key for that. I use those commits A LOT.

I know, I opposed the introduction of this feature initially (and I think
that my concerns were nicely addressed by Junio's suggestion to guard this
feature behind the --autosquash option). Guess what: I was wrong.

And I am really missing the same functionality for the commit message
munging. These days, I find myself using `git commit --allow-empty
--squash=$COMMIT -c $COMMIT` very often, duplicating the first line,
adding an empty line between them, deleting the "squash! " prefix from the
now-third line, and then editing the commit message as I want to. When it
comes to cleaning up the branch via rebase -ki, I simply jump to the empty
line after the squash! line and delete everything before it.

This is as repetitive, tedious and un-fun to me as having to transmogrify
patches from the nice and cozy Git universe into the not-at-all compatible
universe of mails (I congratulate you personally, Peff, for finding a mail
client that works for you. I am still looking for one that does not suck,
Alpine being the least sucky I settled for).

So my idea was to introduce a new --reword= option to `git commit`
that would commit an empty patch and let the user edit the commit message,
later replacing the original one with the new one. This is not *quite* as
nice as I want it, because it makes the changes unobvious. On the other
hand, I would not find a series of sed commands more obvious, in
particular because that limits you in the ways of sed. And, you know,
regexp. I like them, but I know many people cannot really work with them.

> But in practice, most review comments, for the commit message _or_ the
> text, are given in human-readable terms. And as a human, I read and
> apply them in sequence.

So true. I do the very same.

> That pushes work onto the submitter, but saves work from the reviewers,
> who can quickly say "something like this..." without having to worry
> about making a full change, formatting it as a diff, etc.
> 
> I do think that's the right time-tradeoff to be making, as we have more
> submitters than reviewers.

I agree that it is the right trade-off. TBH I was shocked when I learned
how much effort Junio puts into applying my patches. I do not want that. I
want my branch to reflect pretty precisely (modulo sign-off, of course)
what is going to be integrated into Git's source code.

I'd much prefer to resubmit a cleaned-up version, even if it was just the
commit subjects, and be certain that `pu` and my branch are on the same
page.

Instead, Junio puts in a tremendous amount of work, and it does not help
anybody, because the local branches *still* do not have his fixups, and as
a consequence subsequent iterations of the patch series will have to be
fixed up *again*.

Just compare https://github.com/git/git/compare/1fd7e78...6999bc7 to
https://github.com/dscho/git/compare/f8f7adc...3b4494c (the onelines are
enough to show you just how different things are).

I'd much prefer the contributor (me, in this case) to put in a little more
work, and have things consistent. And avoid unnecessary work on both
sides.

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Stefan Beller
On Wed, Aug 3, 2016 at 9:07 AM, Johannes Schindelin
 wrote:
> Hi Junio,
>
> On Wed, 3 Aug 2016, Junio C Hamano wrote:
>
>> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
>>  wrote:
>> >
>> > I disagree, however, with the suggestion to sift through your `pu` branch
>> > and to somehow replace local branches with the commits found there.
>>
>> To be more in line with the "e-mailed patch" workflow, I think what I should
>> do is to send the version I queued with fixups back to the list as follow-up.
>> Just like reviewers review, the maintainer reviews and queues, the original
>> author should be able to work in the same workflow, i.e. reading and applying
>> an improved version of the patch from her mailbox.
>
> You seem to assume that it isn't cumbersome for people like me to extract
> patches out of mails and to replace existing commits using those patches.
>
> So it probably comes as a huge surprise to you to learn that this *is*
> cumbersome for me.

It is also cumbersome for me, because I never had the need to setup a proper
mail client that has the strength to apply patches. The need was not there as
I tend to apply only rarely patches by email, so I can go the painful
way each time.

But if we as a community decide that we bounce emails back and forth,
I (and you)
may have to find a proper email client that is easy to work with, e.g.
one key shortcut
to apply a patch series to the HEAD of your local repository.

>
> I got too used to the ease of git push, git pull with or without --rebase,
> and many other Git commands. Having to transmogrify code changes from
> commits in Git into a completely different universe: plain text patches in
> my mailbox, and back, losing all kinds of data in the process, is just
> not at all that easy. And it costs a lot of time.
>
> In short: if you start "submitting patches" back to me via mail, it does
> not help me. It makes things harder for me. In particular when you add
> your sign-off to every patch and I have to strip it.

You don't have to strip the sign off, as it shows the flow of the patch,
e.g.

Signed-off-by: Johannes Schindelin 
Signed-off-by: Junio C Hamano 
Signed-off-by: Stefan Beller 
Signed-off-by: Johannes Schindelin 
Signed-off-by: Junio C Hamano 

may indicate you proposed a patch, Junio picked it up (and fixed a
typo optionally),
I obtained the patch (via mail, via Git?) improved it, you improved it further
and then Junio took it and merged it upstream.

>
> If you change your workflow, I would humbly request that you do it in a
> way that makes things easier on both sides, not harder.

When attending the Git Merge conference in May, gregkh said roughtly:
"We deliberately waste developers time, because it is not scarce.
Maintainers time is scarce however " and it stuck with me. (and I
am a developer, not a maintainer ;( so at least the kernel community
deems it ok to waste my time).

While that is true for the kernel community, I guess it is also true for the
Git community, unless Junio (and the community) want to appoint a
bunch of maintainer lieutenants, such that they outnumber the number
of developers, e.g. divided by areas of the code:
a refs backend maintainer, a submodule maintainer, ...
or rather by area of usage: a porcelain UI maintainer, a git-on-server
maintainer.

Though Git is not as diverse and large as the kernel, so the horde of
maintainers would step onto each feet quite frequently IMHO.

>
> It would be a totally different matter, of course, if you used the
> branches I publish via my GitHub repository, added fixup! and squash!
> commits, published the result to a public repository and then told me to
> pull from there, that would make things easier. We could even introduce a
> reword! construct, to make the review of the suggested edits of the commit
> message easier. I could easily verify that my branch head agrees with the
> base commit of your branch, I could build proper tooling around this
> workflow, and it would lighten my load.
>
> I guess what I am saying is that we might just as well start using this
> awesome tool to work with code, that tool named "Git".

I think Git itself is for the tracking the code and managing it, e.g. merging,
moving, keeping it. That doesn't quite include modifying and creating code
(e.g. there is no "git edit" command)

If we were to change our workflows drastically, I'd propose to
go a way[1] similar to notedb in Gerrit, or git-series, which defines
a common review format, such that we have a "protocol" how to store
the review data and how to store the progress of potential collaboration
and then we can develop tools against that protocol. Some people
want to have a web UI, whereas others want to have a text only thing
as they are faster keyboard only.

[1] 

Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Junio C Hamano
Jeff King  writes:

> On Wed, Aug 03, 2016 at 08:33:12AM -0700, Junio C Hamano wrote:
>
>> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
>>  wrote:
>> >
>> > I disagree, however, with the suggestion to sift through your `pu` branch
>> > and to somehow replace local branches with the commits found there.
>> 
>> To be more in line with the "e-mailed patch" workflow, I think what I should
>> do is to send the version I queued with fixups back to the list as follow-up.
>> Just like reviewers review, the maintainer reviews and queues, the original
>> author should be able to work in the same workflow, i.e. reading and applying
>> an improved version of the patch from her mailbox.
>
> Leaving aside Dscho's questions of whether pulling patches from email is
> convenient for most submitters (it certainly is for me, but I recognize
> that it is not for many), I would much rather see incremental fixup
> patches from you than whole "here's what I queued" responses.

Ah, yes, I misspoke.  It should be either an incremental diff or
in-line comment to spell out what got changed as a response to the
patch.

I find myself fixing the title the most often, which is part of the
"log message" you pointed out that would not convey well with the
"incremental diff" approach.
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Jeff King
On Wed, Aug 03, 2016 at 09:53:18AM -0700, Junio C Hamano wrote:

> > Leaving aside Dscho's questions of whether pulling patches from email is
> > convenient for most submitters (it certainly is for me, but I recognize
> > that it is not for many), I would much rather see incremental fixup
> > patches from you than whole "here's what I queued" responses.
> 
> Ah, yes, I misspoke.  It should be either an incremental diff or
> in-line comment to spell out what got changed as a response to the
> patch.
> 
> I find myself fixing the title the most often, which is part of the
> "log message" you pointed out that would not convey well with the
> "incremental diff" approach.

I mentioned a micro-format elsewhere in my message. And it certainly is
nice to have something that can be applied in an automatic way. But in
practice, most review comments, for the commit message _or_ the text,
are given in human-readable terms. And as a human, I read and apply them
in sequence.

That pushes work onto the submitter, but saves work from the reviewers,
who can quickly say "something like this..." without having to worry
about making a full change, formatting it as a diff, etc.

I do think that's the right time-tradeoff to be making, as we have more
submitters than reviewers.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Jeff King
On Wed, Aug 03, 2016 at 08:33:12AM -0700, Junio C Hamano wrote:

> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
>  wrote:
> >
> > I disagree, however, with the suggestion to sift through your `pu` branch
> > and to somehow replace local branches with the commits found there.
> 
> To be more in line with the "e-mailed patch" workflow, I think what I should
> do is to send the version I queued with fixups back to the list as follow-up.
> Just like reviewers review, the maintainer reviews and queues, the original
> author should be able to work in the same workflow, i.e. reading and applying
> an improved version of the patch from her mailbox.

Leaving aside Dscho's questions of whether pulling patches from email is
convenient for most submitters (it certainly is for me, but I recognize
that it is not for many), I would much rather see incremental fixup
patches from you than whole "here's what I queued" responses.

The reason is that your fixups may not be the only ones needed. There
may be others on the list that come before or after, and I may even have
already made fixes locally for "v2" that haven't been on the list. If I
haven't made any changes yet, I can throw out my topic, start with what
you queued, and then apply other changes incrementally. But if I have,
then I need to convert yours to a diff, which requires checking out the
same base, applying yours, and running diff. Much easier to get the diff
in the first place. :)

That only covers changes to the code, though. It does not help with
fixups to commit messages. It would be neat to have a microformat for
specifying and applying patches to commit messages.

-Peff
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Johannes Schindelin
Hi Junio,

On Wed, 3 Aug 2016, Junio C Hamano wrote:

> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
>  wrote:
> >
> > I disagree, however, with the suggestion to sift through your `pu` branch
> > and to somehow replace local branches with the commits found there.
> 
> To be more in line with the "e-mailed patch" workflow, I think what I should
> do is to send the version I queued with fixups back to the list as follow-up.
> Just like reviewers review, the maintainer reviews and queues, the original
> author should be able to work in the same workflow, i.e. reading and applying
> an improved version of the patch from her mailbox.

You seem to assume that it isn't cumbersome for people like me to extract
patches out of mails and to replace existing commits using those patches.

So it probably comes as a huge surprise to you to learn that this *is*
cumbersome for me.

I got too used to the ease of git push, git pull with or without --rebase,
and many other Git commands. Having to transmogrify code changes from
commits in Git into a completely different universe: plain text patches in
my mailbox, and back, losing all kinds of data in the process, is just
not at all that easy. And it costs a lot of time.

In short: if you start "submitting patches" back to me via mail, it does
not help me. It makes things harder for me. In particular when you add
your sign-off to every patch and I have to strip it.

If you change your workflow, I would humbly request that you do it in a
way that makes things easier on both sides, not harder.

It would be a totally different matter, of course, if you used the
branches I publish via my GitHub repository, added fixup! and squash!
commits, published the result to a public repository and then told me to
pull from there, that would make things easier. We could even introduce a
reword! construct, to make the review of the suggested edits of the commit
message easier. I could easily verify that my branch head agrees with the
base commit of your branch, I could build proper tooling around this
workflow, and it would lighten my load.

I guess what I am saying is that we might just as well start using this
awesome tool to work with code, that tool named "Git".

Ciao,
Dscho
--
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: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Junio C Hamano
On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
 wrote:
>
> I disagree, however, with the suggestion to sift through your `pu` branch
> and to somehow replace local branches with the commits found there.

To be more in line with the "e-mailed patch" workflow, I think what I should
do is to send the version I queued with fixups back to the list as follow-up.
Just like reviewers review, the maintainer reviews and queues, the original
author should be able to work in the same workflow, i.e. reading and applying
an improved version of the patch from her mailbox.
--
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


patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

2016-08-03 Thread Johannes Schindelin
Hi Junio,

On Tue, 2 Aug 2016, Junio C Hamano wrote:

> So either I should change my workflow and mention any and all
> typofixes in my review comments (which consumes the review
> bandwidth), or I should force patch authors to do the "fetch from
> 'pu' and replace" somehow to avoid this kind of back-and-forth.

It never occurred to me that I should replace any of my local branches
with versions you have in `pu`. For several reasons:

- just as you do not pull from me, lest patches enter your repository that
  you have not reviewed, I do not simply pull from you,

- I thought the point of avoiding GitHub in the review process at all
  costs and require everybody to go via the mailing list instead was to
  keep the review process open? These silent changes fly in the face of
  that,

- I agree that the mail-based process is cumbersome and error-prone, but
  we won't fix it by asking contributors to dig through the `pu` of the
  day, somehow stay up-to-date with possibly more silent typofixes the
  next day, somehow manage to figure out what changes exactly were
  introduced to their patches, and replace their local work.

- even if we asked for all that trouble, the commits in `pu` are all
  signed off by you. These sign-offs would have to be stripped out
  tediously when making local changes.

In short, I agree that our patch submission process is a saber tooth tiger
that still reflects pre-Git times. While we use Git's tools, the workflow
really tries to cut out Git as much as possible, in favor of pure mails
with non-corrupted, non-HTML patches in them, a charmingly anachronistic
requirement until you try to use state-of-the-art mail clients to send
them.

I disagree, however, with the suggestion to sift through your `pu` branch
and to somehow replace local branches with the commits found there.

I guess it is time to revisit our patch submission process if it does not
even work between the two of us.

Ideally, we would come up with a process that

- makes everything easier for maintainers and contributors alike,

- tracks the history of the patch iterations (answering the question "what
  changed between iterations?"),

- *actually* integrates with Git (to see what I mean, try to find the
  commit corresponding to a given mail containing a patch, and then try to
  find the previous iteration's version of the same commit, and weep),

- provides machine-readable metadata about the context, e.g. to jump back
  and forth between the full file contents and the patch, or to indicate
  the dependency on another branch,

- facilitates "back contributions", i.e. letting contributors accept
  changes suggested by reviewers *with minimal effort*.

- uses Git itself as much as possible, i.e. no additional tools written in
  "you must learn this new language, it's awesome, believe me, it's huge"

The biggest obstacles I see are 1) the integration with the mailing list
(which is ironic because contributing via the list used to be a boon, not
a burden) and 2) maintaining the integrity between what has been reviewed
and what is actually in the branch.

This is nothing we will solve overnight, of course. But I think we will
have to fix this.

Food for thought.
Dscho
--
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