[PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Jon Seymour]

The documentation of the dcommit subcommand is reworded to clarify that
the optional argument refers to a git branch, not an SVN branch.

The discussion of the optional argument is put into its own paragraph
as is the guidance about using 'dcommit' in preference to 'set-tree'.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 Documentation/git-svn.txt | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 34ee785..27c4840 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -189,18 +189,20 @@ and have no uncommitted changes.
 	last fetched commit from the upstream SVN.
 
 'dcommit'::
-	Commit each diff from a specified head directly to the SVN
+	Commit each diff from the current branch directly to the SVN
 	repository, and then rebase or reset (depending on whether or
 	not there is a diff between SVN and head).  This will create
 	a revision in SVN for each commit in git.
 	It is recommended that you run 'git svn' fetch and rebase (not
 	pull or merge) your commits against the latest changes in the
 	SVN repository.
-	An optional revision or branch argument may be specified, and
-	causes 'git svn' to do all work on that revision/branch
-	instead of HEAD.
-	This is advantageous over 'set-tree' (below) because it produces
-	cleaner, more linear history.
++
+When an optional git branch name (or a git commit object name)
+is specified as an argument, the subcommand works on the specified
+branch, not on the current branch.
++
+Use of 'dcommit' is preferred to 'set-tree' (below) because it produces
+cleaner, more linear history.
 +
 --no-rebase;;
 	After committing, do not rebase or reset.
-- 
1.7.10.1.514.ge33c7ea

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Jon Seymour]

On Mon, May 14, 2012 at 4:53 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> The documentation of the dcommit subcommand is reworded to clarify that
> the optional argument refers to a git branch, not an SVN branch.
>
> The discussion of the optional argument is put into its own paragraph
> as is the guidance about using 'dcommit' in preference to 'set-tree'.
>
> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
> ---
>  Documentation/git-svn.txt | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
> index 34ee785..27c4840 100644
> --- a/Documentation/git-svn.txt
> +++ b/Documentation/git-svn.txt
> @@ -189,18 +189,20 @@ and have no uncommitted changes.
>        last fetched commit from the upstream SVN.
>
>  'dcommit'::
> -       Commit each diff from a specified head directly to the SVN
> +       Commit each diff from the current branch directly to the SVN
>        repository, and then rebase or reset (depending on whether or
>        not there is a diff between SVN and head).  This will create
>        a revision in SVN for each commit in git.
>        It is recommended that you run 'git svn' fetch and rebase (not
>        pull or merge) your commits against the latest changes in the
>        SVN repository.
> -       An optional revision or branch argument may be specified, and
> -       causes 'git svn' to do all work on that revision/branch
> -       instead of HEAD.
> -       This is advantageous over 'set-tree' (below) because it produces
> -       cleaner, more linear history.
> ++
> +When an optional git branch name (or a git commit object name)
> +is specified as an argument, the subcommand works on the specified
> +branch, not on the current branch.
> ++
> +Use of 'dcommit' is preferred to 'set-tree' (below) because it produces
> +cleaner, more linear history.
>  +

Mmmm. I wonder if I should have left the original wording of this
paragraph, but switched the order with respect to the disucssion of
the optional argument, so that the text reads:

    It is recommended that you run 'git svn' fetch and rebase (not
pull or merge) your commits against the latest changes in the  SVN
repository.
    This is advantageous over'set-tree' (below) because it produces
cleaner, more linear history.

    When an optional git branch name (or a git commit object name) is
specified as an argument, the subcommand works on
    the specified branch, not on the current branch.

>  --no-rebase;;
>        After committing, do not rebase or reset.
> --
> 1.7.10.1.514.ge33c7ea
>

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Junio C Hamano]

Jon Seymour <jon.seymour@gmail.com> writes:

>>  'dcommit'::
>> -       Commit each diff from a specified head directly to the SVN
>> +       Commit each diff from the current branch directly to the SVN
>>        repository, and then rebase or reset (depending on whether or
>>        not there is a diff between SVN and head).  This will create
>>        a revision in SVN for each commit in git.
>>        It is recommended that you run 'git svn' fetch and rebase (not
>>        pull or merge) your commits against the latest changes in the
>>        SVN repository.
>> -       An optional revision or branch argument may be specified, and
>> -       causes 'git svn' to do all work on that revision/branch
>> -       instead of HEAD.
>> -       This is advantageous over 'set-tree' (below) because it produces
>> -       cleaner, more linear history.
>> ++
>> +When an optional git branch name (or a git commit object name)
>> +is specified as an argument, the subcommand works on the specified
>> +branch, not on the current branch.
>> ++
>> +Use of 'dcommit' is preferred to 'set-tree' (below) because it produces
>> +cleaner, more linear history.
>>  +
>
> Mmmm. I wonder if I should have left the original wording of this
> paragraph, but switched the order with respect to the disucssion of
> the optional argument, so that the text reads:
>
>     It is recommended that you run 'git svn' fetch and rebase (not
> pull or merge) your commits against the latest changes in the  SVN
> repository.
>     This is advantageous over'set-tree' (below) because it produces
> cleaner, more linear history.
>
>     When an optional git branch name (or a git commit object name) is
> specified as an argument, the subcommand works on
>     the specified branch, not on the current branch.

Probably not, but the order in the [PATCH v2] is not optimal, either.  I
would think it would read nicer to give "what it does" and then
"recommendations", i.e.

 - (what it does) Commit each diff from the current branch of git to svn.
   With an optional git branch name, the subcommand works on that branch.

 - (recommendations)
   . Do fetch and rebase, not pull and merge.  
   . Prefer dcommit over set-tree.

The only reason why the original is not ordered that way, as far as I can
tell, is because "It is recommended that" was part of an existing paragraph
when dd31da2 (git-svn: allow dcommit to take an alternate head, 2006-12-12)
added "An optional ...", so it was tucked after existing paragraph without
reading the resulting whole to see if "at the very end" was the best place.

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Jon Seymour]

On Tue, May 15, 2012 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> The only reason why the original is not ordered that way, as far as I can
> tell, is because "It is recommended that" was part of an existing paragraph
> when dd31da2 (git-svn: allow dcommit to take an alternate head, 2006-12-12)
> added "An optional ...", so it was tucked after existing paragraph without
> reading the resulting whole to see if "at the very end" was the best place.

Re-reading this, I don't think the advice:

    "It is recommended that you run 'git svn' fetch and rebase (not
    pull or merge) your commits against the latest changes in the =C2=A0SVN
    repository."

really belongs in the description of dcommit at all.

My proposed rewording for dcommit is:

"Commit each diff from the current branch directly to the SVN
repository, and then rebase or reset (depending on whether or not
there is a diff between SVN and head). This will create a revision in
SVN for each commit in git.

When an optional git branch name (or a git commit object name) is
specified as an argument, the subcommand works on the specified
branch, not on the current branch.

Use of dcommit is preferred to set-tree (below)."

There is a section in "REBASE VS. PULL/MERGE" which perhaps could be
reworded to include any points in the text I propose to omit from
'dcommit' which aren't already covered there.

jon.

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Junio C Hamano]

Jon Seymour <jon.seymour@gmail.com> writes:

> On Tue, May 15, 2012 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> The only reason why the original is not ordered that way, as far as I can
>> tell, is because "It is recommended that" was part of an existing paragraph
>> when dd31da2 (git-svn: allow dcommit to take an alternate head, 2006-12-12)
>> added "An optional ...", so it was tucked after existing paragraph without
>> reading the resulting whole to see if "at the very end" was the best place.
>
> Re-reading this, I don't think the advice:
>
>     "It is recommended that you run 'git svn' fetch and rebase (not
>     pull or merge) your commits against the latest changes in the =C2=A0SVN
>     repository."
>
> really belongs in the description of dcommit at all.

I tend to agree.  Opinions from git-svn experts?

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
> > Re-reading this, I don't think the advice:
> >
> >     "It is recommended that you run 'git svn' fetch and rebase (not
> >     pull or merge) your commits against the latest changes in the =C2=A0SVN
> >     repository."
> >
> > really belongs in the description of dcommit at all.
> 
> I tend to agree.  Opinions from git-svn experts?

I agree, too.   It should be moved to/reworded for the
"REBASE VS. PULL/MERGE" section instead.

Re: [PATCH v2] git-svn: clarify the referent of dcommit's optional argument

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> Jon Seymour <jon.seymour@gmail.com> writes:
>
>> On Tue, May 15, 2012 at 2:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> The only reason why the original is not ordered that way, as far as I can
>>> tell, is because "It is recommended that" was part of an existing paragraph
>>> when dd31da2 (git-svn: allow dcommit to take an alternate head, 2006-12-12)
>>> added "An optional ...", so it was tucked after existing paragraph without
>>> reading the resulting whole to see if "at the very end" was the best place.
>>
>> Re-reading this, I don't think the advice:
>>
>>     "It is recommended that you run 'git svn' fetch and rebase (not
>>     pull or merge) your commits against the latest changes in the =C2=A0SVN
>>     repository."
>>
>> really belongs in the description of dcommit at all.
>
> I tend to agree.  Opinions from git-svn experts?

I agree that this is not dcommit-specific advice.  Perhaps there should
be a longer section that explains in detail what is rebased how and thus
what the limitations of dcommit are.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch