git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Update git rebase documentation to clarify HEAD behavior
@ 2016-10-27  4:13 Cody Sehl
  2016-10-27  6:47 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Cody Sehl @ 2016-10-27  4:13 UTC (permalink / raw)
  To: git

The first few paragraphs in the git-rebase.txt documentation lay out the steps git takes during a rebase:
1. everything from `<upstream>..HEAD` is saved to a temporary area
2. `HEAD` is set to `<upstream>`
3. the changes held in the temporary area are applied one by one in order on top of the new `HEAD`

The second step was described using the phrase `The current branch is reset to <upstream>`, which is true (because `HEAD` == current branch), but not clear.
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..c47ca11 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -33,7 +33,7 @@ of commits that would be shown by `git log <upstream>..HEAD`; or by
 description on `--fork-point` below); or by `git log HEAD`, if the
 `--root` option is specified.
 
-The current branch is reset to <upstream>, or <newbase> if the
+HEAD is reset to <upstream>, or <newbase> if the
 --onto option was supplied.  This has the exact same effect as
 `git reset --hard <upstream>` (or <newbase>).  ORIG_HEAD is set
 to point at the tip of the branch before the reset.

--
https://github.com/git/git/pull/301

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Update git rebase documentation to clarify HEAD behavior
  2016-10-27  4:13 [PATCH] Update git rebase documentation to clarify HEAD behavior Cody Sehl
@ 2016-10-27  6:47 ` Junio C Hamano
  2016-10-27 14:06   ` Philip Oakley
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2016-10-27  6:47 UTC (permalink / raw)
  To: Cody Sehl; +Cc: git

Cody Sehl <cody.sehl@gmail.com> writes:

> The first few paragraphs in the git-rebase.txt documentation lay out the steps git takes during a rebase:
> 1. everything from `<upstream>..HEAD` is saved to a temporary area
> 2. `HEAD` is set to `<upstream>`
> 3. the changes held in the temporary area are applied one by one in order on top of the new `HEAD`
>
> The second step was described using the phrase `The current branch is reset to <upstream>`, which is true (because `HEAD` == current branch), but not clear.
> ---

Please wrap your lines to reasonable lengths like 70 columns or so.
Please sign off your patch.

>  Documentation/git-rebase.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index de222c8..c47ca11 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -33,7 +33,7 @@ of commits that would be shown by `git log <upstream>..HEAD`; or by
>  description on `--fork-point` below); or by `git log HEAD`, if the
>  `--root` option is specified.
>  
> -The current branch is reset to <upstream>, or <newbase> if the
> +HEAD is reset to <upstream>, or <newbase> if the
>  --onto option was supplied.  This has the exact same effect as
>  `git reset --hard <upstream>` (or <newbase>).  ORIG_HEAD is set
>  to point at the tip of the branch before the reset.

This is describing an ancient behaviour before 6fd2f5e60d ("rebase:
operate on a detached HEAD", 2007-11-08) in v1.5.4 timeframe.  We
apparently failed to update the description.  

This depends on the desired technical detail of the description, but
a correct rewrite would be "HEAD is detached at <upstream>, or
<newbase>, and ORIG_HEAD is set to point at the tip of the branch
before this happens".  Detaching the HEAD at <upstream> no longer
has the same effect as "git reset --hard <upstream>" (or <newbase>),
so that sentence must go.  It was the primary point of the ancient
change at 6fd2f5e60d after all.

And then there is a new step (to be numbered 4. in your description
in the proposed log message), which updates the tip of the branch to
the resulting HEAD (after replaying all these changes) and check the
branch out, which needs to be added.  Perhaps after "one by one, in
order."  Oh, the mention of "reapplied to the current branch" also
needs to be updated to "reapplied to the detached HEAD", too.

On the other hand, if we do not aim for that deep level of technical
correctness, but want to tell a white lie to make it easier to
understand at the conceptual level to new readers who haven't
grasped the detached HEAD, then the current description is fine.  By
bringing up "HEAD", you seem to be aiming for techincal correctness
(which I tend to agree is a good direction to go in this part of the
documentation), so the existing text needs a bit more work than your
patch to be brought to the modern world.



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Update git rebase documentation to clarify HEAD behavior
  2016-10-27  6:47 ` Junio C Hamano
@ 2016-10-27 14:06   ` Philip Oakley
  0 siblings, 0 replies; 3+ messages in thread
From: Philip Oakley @ 2016-10-27 14:06 UTC (permalink / raw)
  To: Junio C Hamano, Cody Sehl; +Cc: git

From: "Junio C Hamano" <gitster@pobox.com>
> Cody Sehl <cody.sehl@gmail.com> writes:
>
>> The first few paragraphs in the git-rebase.txt documentation lay out the 
>> steps git takes during a rebase:
>> 1. everything from `<upstream>..HEAD` is saved to a temporary area
>> 2. `HEAD` is set to `<upstream>`
>> 3. the changes held in the temporary area are applied one by one in order 
>> on top of the new `HEAD`
>>
>> The second step was described using the phrase `The current branch is 
>> reset to <upstream>`, which is true (because `HEAD` == current branch), 
>> but not clear.
>> ---
>
> Please wrap your lines to reasonable lengths like 70 columns or so.
> Please sign off your patch.
>
>>  Documentation/git-rebase.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index de222c8..c47ca11 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -33,7 +33,7 @@ of commits that would be shown by `git log 
>> <upstream>..HEAD`; or by
>>  description on `--fork-point` below); or by `git log HEAD`, if the
>>  `--root` option is specified.
>>
>> -The current branch is reset to <upstream>, or <newbase> if the
>> +HEAD is reset to <upstream>, or <newbase> if the
>>  --onto option was supplied.  This has the exact same effect as
>>  `git reset --hard <upstream>` (or <newbase>).  ORIG_HEAD is set
>>  to point at the tip of the branch before the reset.
>
> This is describing an ancient behaviour before 6fd2f5e60d ("rebase:
> operate on a detached HEAD", 2007-11-08) in v1.5.4 timeframe.  We
> apparently failed to update the description.
>
> This depends on the desired technical detail of the description, but
> a correct rewrite would be "HEAD is detached at <upstream>, or
> <newbase>, and ORIG_HEAD is set to point at the tip of the branch
> before this happens".  Detaching the HEAD at <upstream> no longer
> has the same effect as "git reset --hard <upstream>" (or <newbase>),
> so that sentence must go.  It was the primary point of the ancient
> change at 6fd2f5e60d after all.
>
> And then there is a new step (to be numbered 4. in your description
> in the proposed log message), which updates the tip of the branch to
> the resulting HEAD (after replaying all these changes) and check the
> branch out, which needs to be added.  Perhaps after "one by one, in
> order."  Oh, the mention of "reapplied to the current branch" also
> needs to be updated to "reapplied to the detached HEAD", too.
>
> On the other hand, if we do not aim for that deep level of technical
> correctness, but want to tell a white lie to make it easier to
> understand at the conceptual level to new readers who haven't
> grasped the detached HEAD, then the current description is fine.  By
> bringing up "HEAD", you seem to be aiming for techincal correctness
> (which I tend to agree is a good direction to go in this part of the
> documentation), so the existing text needs a bit more work than your
> patch to be brought to the modern world.
>

A third option is to start with a short "Conceptually, .." description which 
gives that summary overview, and then have a few "In detail, .." paragraphs 
that cover the ORIG_HEAD and Detatched HEAD, etc., that users should at 
least be aware of (so they know they can come back and read the gory details 
when they need it;-)
--
Philip 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-10-27 18:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27  4:13 [PATCH] Update git rebase documentation to clarify HEAD behavior Cody Sehl
2016-10-27  6:47 ` Junio C Hamano
2016-10-27 14:06   ` Philip Oakley

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).