[PATCH v9] Documentation/remote-helpers: Add invocation section

Previous thread: [PATCH v8] Documentation/remote-helpers: Add invocation section by Ramkumar Ramachandra on Tuesday, April 6, 2010 - 10:44 pm. (1 message)

Next thread: [PATCH v2] Documentation/remote-helpers: Fix typos and improve language by Ramkumar Ramachandra on Wednesday, April 7, 2010 - 1:51 am. (1 message)
From: Ramkumar Ramachandra
Date: Tuesday, April 6, 2010 - 10:57 pm

Add an 'Invocation' section to specify what the command line arguments
mean. Also include a link to git-remote in the 'See Also' section.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Changes: The second argument may be an arbitrary string as
 Jonathan pointed out.

 Documentation/git-remote-helpers.txt |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index 7a5569c..b0e7d54 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -7,7 +7,7 @@ git-remote-helpers - Helper programs to interact with
remote repositories

 SYNOPSIS
 --------
-'git remote-<transport>' <remote>
+'git remote-<transport>' <repository> [<URL>]

 DESCRIPTION
 -----------
@@ -38,6 +38,34 @@ transport protocols, such as 'git-remote-http',
'git-remote-https',
 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
 'fetch', 'option', and 'push'.

+INVOCATION
+----------
+
+Remote helper programs are invoked with one or (optionally) two
+arguments. The first argument specifies a remote repository as in git;
+it is either the name of a configured remote or a URL. The second
+argument specifies a URL; it is usually of the form
+'<transport>://<address>', but any arbitrary string is possible.
+
+When git encounters a URL of the form '<transport>://<address>', where
+'<transport>' is a protocol that it cannot handle natively, it
+automatically invokes 'git remote-<transport>' with the full URL as
+the second argument. If such a URL is encountered directly on the
+command line, the first argument is the same as the second, and if it
+is encountered in a configured remote, the first argument is the name
+of that remote.
+
+A URL of the form '<transport>::<address>' explicitly instructs git to
+invoke 'git remote-<transport>' with '<address>' as the second
+argument. If such a URL is encountered ...
From: Ramkumar Ramachandra
Date: Wednesday, April 7, 2010 - 1:56 am

Hi Junio,

I see that you've applied v8 of my patch to pu, but there's a small
change in this revision:

diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index 8838388..6ffc0da 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -44,9 +44,8 @@ INVOCATION
 Remote helper programs are invoked with one or (optionally) two
 arguments. The first argument specifies a remote repository as in git;
 it is either the name of a configured remote or a URL. The second
-argument specifies a URL of the form '<transport>://<address>' or
-'<transport>::<address>', where '<address>' may be an arbitrary
-string.
+argument specifies a URL; it is usually of the form
+'<transport>://<address>', but any arbitrary string is possible.

 When git encounters a URL of the form '<transport>://<address>', where
 '<transport>' is a protocol that it cannot handle natively, it

-- Ram
--

From: Junio C Hamano
Date: Wednesday, April 7, 2010 - 8:05 am

You keep sending a broken patch.  I've queued v7 and replaced with v8 and
both times I had to fix them in my editor.

I'm not replacing v8 with this tonight, as my pre-push test cycles are
already running (finished in one vm and continuing in another).  Please
check what I queued and make sure your future patches applies on top of
what I have.

Thanks.

--

From: Ramkumar Ramachandra
Date: Wednesday, April 7, 2010 - 9:50 am

Oh, thanks for telling me- I should make sure that my future patches
aren't broken.
Can someone tell me what I'm doing wrong? Earlier, Junio also had a
problem with one of my patches being corrupt. I used git-imap-send and


Right, so you can just squash them. Also, I think I'm sending in
patches too fast so I'll slow down and rebase them on top of `pu`
every time.
Do tell me anything else that I can do to make my patches easier to
review and apply in future.

-- Ram
--

From: Junio C Hamano
Date: Wednesday, April 7, 2010 - 10:23 am

In the above procedure, up to imap-send, everything would be fine, but I
heard GMail web interface still wraps lines before sending the result out.

Gmail section in Documentation/SubmittingPatches says:

  GMail does not appear to have any way to turn off line wrapping in the web
  interface, so this will mangle any emails that you send.  You can however
  use any IMAP email client to connect to the google imap server, and forward
  the emails through that.  Just make sure to disable line wrapping in that
  email client.  Alternatively, use "git send-email" instead.

This is a tangent, but I think this can still be further improved.  What
it says is not incorrect per-se, but the "can however" gives a false hope
to people who have been burned by the web interface.  The fact it lists
are:

 - GMail web interface will wrap lines and corrupt patches, no matter what
   you do;

 - With imap-send, you can stuff messages to your outbox at GMail, and
   using IMAP client that does _not_ corrupt messages, you can send things
   out.

Combined together, these mean that you lose even if you preprare your
outbox with imap-send if you send out the end result from their web
interface.

At the minimum, we could add a parenthesized comment like this.

  the emails through that.  Just make sure to disable line wrapping in
  that email client (again, there is no way to tell GMail web interface to
  do so).


--

From: Ramkumar Ramachandra
Date: Wednesday, April 7, 2010 - 10:58 am

Actually the last paragraph about "git imap-send" clearly states that
the web interface _can_ be used. I just used another email client to
find out when lines break and when they don't- I've accordingly
written a patch for Documentation/SubmittingPatches clarifying this.

-- Ram
--

From: Jonathan Nieder
Date: Wednesday, April 7, 2010 - 3:49 pm

Thank you for this.  FWIW I find it useful when discussing a piece of
code or documentation to bounce back and forth multiple versions of
small fragments.  Then once the discussion has settled down, it can be
useful to see the big picture again, with the new changes
incorporated.

If you want to make sure the latest version of a patch is always
available, that is a noble goal, too, but I think a frequently-
rebased public branch for your patch series is a better way to achieve
that.

Unrelated: in case you are interested, the git-resurrect.sh script
from contrib can extract topic branches from pu, if you want to see
Junio’s topic branch for your patch.

Cheers,
Jonathan
--

From: Junio C Hamano
Date: Wednesday, April 7, 2010 - 4:02 pm

It might look unrelated, but I found it the most helpful information in
your message ;-) I really hate it when people base their patches on 'pu'.

At least it would be helpful for me if people based their follow-up
patches on top of their own topics.
--

From: Ramkumar Ramachandra
Date: Wednesday, April 7, 2010 - 9:54 pm

Hi,


Got it. In future, I'll post diffs for smaller changes hopefully in
the same email thread, and complete patches for larger changes in a

Ah, I already do that, but my branch is based on `pu`. What should I do?
http://github.com/artagnon/git


Can I see the topic branch corresponding to my patches? If this is
possible, I can avoid the more painful procedure of extracting it from
the latest pu using git-resurrect.sh.

-- Ram
--

From: Jonathan Nieder
Date: Wednesday, April 7, 2010 - 10:03 pm

I tend to do the extraction by hand:

 1. git log --first-parent origin/pu
 2. Search for "rr/".
 3. git branch -f rr/whatever <relevant sha1>

Maybe it would be nice to have a script that does just that instead of
the more costly "git resurrect -m rr/whatever".

Cheers,
Jonathan
--

From: Junio C Hamano
Date: Wednesday, April 7, 2010 - 10:14 pm

Run:

    $ git log --first-parent --oneline origin/master..origin/pu

and find "Merge branch 'rr/remote-helper-doc'" in the output.  The second
parent of that merge commit (e.g. 6144af5^2 == ad466d1) is the tip of your
topic.  You can verify it with:

    $ git log -p origin/master..ad466d1
    $ git checkout -b remote-helper-doc ad466d1
    ... build further on it ...

--

From: Ramkumar Ramachandra
Date: Saturday, April 10, 2010 - 5:24 am

Thanks for the excellent instructions. I'm just curious: why didn't
you include these instructions in your rewrite of SubmittingPatches?

-- Ram
--

From: Sverre Rabbelier
Date: Thursday, April 8, 2010 - 11:52 am

Heya,


Hmmm, perhaps we should update SubmittingPatches to say something
about that? The section that talks about what to base your patch
against [0] is not very explicit in that aspect.

[0] http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/SubmittingPatches#l106

-- 
Cheers,

Sverre Rabbelier
--

From: Junio C Hamano
Date: Thursday, April 8, 2010 - 1:01 pm

Ok, here is a rough draft.

 Documentation/SubmittingPatches |   52 ++++++++++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c686f86..1d403ee 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -53,6 +53,37 @@ But the patch submission requirements are a lot more relaxed
 here on the technical/contents front, because the core GIT is
 thousand times smaller ;-).  So here is only the relevant bits.
 
+(0) Decide what to base your work on.
+
+The general principle is always to base your work on the oldest branch
+that your change is relevant to.
+
+ - A fix for a bug that has been with git from older releases should be
+   included in both the upcoming feature release and the current
+   maintenance release.  Try to base your work on the 'maint' branch.  A
+   work to kill a bug that is in 'master' but not in 'maint' should be
+   based on 'master'.
+
+ - A fix for a bug that is not yet in 'master' is the best bug to kill.
+   If you can find the topic that introduces the regression, base your
+   work on the tip of the topic.  "log --first-parent master..pu" would be
+   a good way to find the tips of topic branches.
+
+ - A new feature should be based on the 'master' branch in general.
+
+ - If your new feature depends on some other topics that are not in
+   'master' yet, and if it relies only on one topic, base your work on the
+   tip of that topic.  If it depends on too many topics that are not in
+   'master', you can privately start working on 'next' or even 'pu' and
+   send out your patches for discussion, but it is possible that your
+   maintainer may ask you to wait and rebase your changes on 'master'
+   after some of the larger topics your topic depends on graduate to
+   'master'.
+
+ - Base corrections and enhancements on a topic that are not in 'master'
+   yet but already merged to 'next' on the tip ...
From: Sverre Rabbelier
Date: Thursday, April 8, 2010 - 1:21 pm

Heya,



... I'm curious as to why you removed these two statements.

-- 
Cheers,

Sverre Rabbelier
--

From: Junio C Hamano
Date: Thursday, April 8, 2010 - 1:45 pm

The rewritten paragraph already covers these two points, no?  Proposals
can be cc:ed to me only if what I did in the past is relevant to the
proposal, meaning "treat the maintainer just like any other contributors
and reviewers", and it applies to all the codebase, not just the contrib/
area.
--

From: Michael J Gruber
Date: Thursday, April 8, 2010 - 7:06 pm

I'm wondering how necessary that flipping of to and cc is. It means one
has to switch one's send-email config between RFCs and actual patches.
It also means I should send fewer patches to you (Junio) directly (in
addition to cc'ing the list), which is probably the intention :)
OK, I've learned about aliasesfile (and wondered about the different
--

From: Junio C Hamano
Date: Friday, April 9, 2010 - 9:59 pm

This was my thinko.  "An ideal patch flow" says the final submission still
goes to the list with maintainer on the Cc: line, and that is fine with

That, and more importantly, less uncooked patches hitting my mailbox with
me as an addressee, was an important goal.  I need to deal with patch
e-mails in three different ways:

 (0) just read as a bystander without much interesting input from my side;
 (1) read and comment for improvement; or
 (2) act on it by applying.

I was hoping if we can have some way for me to sift (2) from others
without adding extra burden on the contributors.

One way to reduce (1) I've been experimenting with is not to comment on
too many threads.  The theory is that the initial rfc patch hopefully does
not have my address on To/Cc, but once I comment on a patch, follow-up
patches will be posted with address of everybody involved in the
discussion Cc:ed to the thread (which by the way is a good practice and I
do not want people to break it), and my "grep for patch messages with my
address on it" trick to find the finalized patches will not work well.

But that strategy does not work very well for another reason: we lose one
reviewer if I try to comment on patches as little as possible, and we do
not have enough people who read other's patches and help polishing to
afford that.
--

From: Ramkumar Ramachandra
Date: Saturday, April 10, 2010 - 5:36 am

Hi,

I like the idea, but I think it's too verbose. Perhaps something along
these lines?

1. In general, base your patches on 'master'.
2. If your patch is a bugfix try to base it on 'maint'. If the bug's
in 'master, but not in 'maint' also, base it on 'master'.
3. If you're working on a feature, some of whose patches are already
in 'pu', base your work on the tip of the last commit in your topic
branch (See THIS). If it's a minor correction, you might want to add a
note asking the maintainer to squash it into the previous commit.
4. If you're working on an elaborate feature that depends on many
commits in 'pu', maintain a public branch based on 'pu', and
periodically post patches to the mailing list for feedback (all based
on 'pu'). You might have to rebase/ wait before it's finally merged.


+ "The grandparent of this merge commit is your latest commit in this
topic. This is the commit you should base your patch on."

-- Ram
--

Previous thread: [PATCH v8] Documentation/remote-helpers: Add invocation section by Ramkumar Ramachandra on Tuesday, April 6, 2010 - 10:44 pm. (1 message)

Next thread: [PATCH v2] Documentation/remote-helpers: Fix typos and improve language by Ramkumar Ramachandra on Wednesday, April 7, 2010 - 1:51 am. (1 message)