[wwwdocs] Recommend reviewing local changes before pushing them

Message ID 20200114133702.GA346366@redhat.com
State New
Headers show
Series
  • [wwwdocs] Recommend reviewing local changes before pushing them
Related show

Commit Message

Jonathan Wakely Jan. 14, 2020, 1:37 p.m.
I really think people should be reviewing what they're about to push
before doing it.

OK for wwwdocs?
commit 2c54cb9e98fe67096b62718d8dbd3b2ca485ff89
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 13:34:02 2020 +0000

    Recommend reviewing local changes before pushing them

Comments

Richard Earnshaw Jan. 14, 2020, 1:39 p.m. | #1
On 14/01/2020 13:37, Jonathan Wakely wrote:
> I really think people should be reviewing what they're about to push

> before doing it.

> 

> OK for wwwdocs?

> 


I'd recommend

git push origin HEAD:<target-branch>

rather than just 'git push'

Otherwise, OK.

R.
Richard Biener Jan. 14, 2020, 2:08 p.m. | #2
On Tue, Jan 14, 2020 at 2:37 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>

> I really think people should be reviewing what they're about to push

> before doing it.

>

> OK for wwwdocs?


So I figure that first doing the git push with -n -v and then reviewing
the pushed changes (cut&paste the revision range) with git show
is a workflow easy to remember and less "implicit" than your wording
(I dont' understand what I actually should _do_ with to review the
changes for your description)

Richard.

>
Jonathan Wakely Jan. 14, 2020, 2:28 p.m. | #3
On 14/01/20 15:08 +0100, Richard Biener wrote:
>On Tue, Jan 14, 2020 at 2:37 PM Jonathan Wakely <jwakely@redhat.com> wrote:

>>

>> I really think people should be reviewing what they're about to push

>> before doing it.

>>

>> OK for wwwdocs?

>

>So I figure that first doing the git push with -n -v and then reviewing

>the pushed changes (cut&paste the revision range) with git show

>is a workflow easy to remember and less "implicit" than your wording


"cut&paste the revision range" seems awfully tedious to me ;-)

The equivalent is 'git show @{u}..' or if you don't want to remember
the @{u} thing (aka @{upstream}), you can name the branch explicitly:

   git show origin/master..

or even more explicitly (but identical):

   git show origin/master..HEAD

>(I dont' understand what I actually should _do_ with to review the

>changes for your description)


If you run the commands, isn't it obvious? (Genuine question)

master wwwdocs$ git log @{u}..
commit 0eecf8a69a52cd27faeea64c8269a6df3f8db855 (HEAD -> master)
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 13:34:02 2020 +0000

     Recommend reviewing local changes before pushing them

commit a449e14116456cf66ea15d4aca203a50b55f6e20
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 13:16:12 2020 +0000

     Fix indentation of .ssh/config snippet

Here git log shows two local commits. If I push, they'll both be
pushed. Is that what I meant to do, or did I forget that a449e1411 was
also in my local branch?

You should check that only the commits you expect are shown in the
log.

You can also just do "git log" and check which commits are more recent
than the one shown as the upstream:

$ git log
commit 9e502f6deae9f821bd7079aad5f98a4f3bae15cf (HEAD -> master)
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 13:34:02 2020 +0000

     Recommend reviewing local changes before pushing them

commit 83f833b74f2dada4235f1b68b2e3cab5e5bba757
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 13:16:12 2020 +0000

     Fix indentation of .ssh/config snippet

commit 10463a79371068a0b32d8babefb9cf2ee409f4d1 (origin/master, origin/HEAD)
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Tue Jan 14 13:48:19 2020 +0000

     [arm] Document -mpure-code support for v6m in gcc-10

The one marked (origin/master, origin/HEAD) is the commit that is
upstream, the ones before it are local.

Maybe I should just say use 'git status' and/or 'git log' and check
that your local branch doesn't contain anything unexpected.

The other command shows the diff between your local branch and
upstream. Check the diff only contains the differences you expect it
to.
Jonathan Wakely Jan. 14, 2020, 2:38 p.m. | #4
On 14/01/20 13:39 +0000, Richard Earnshaw wrote:
>On 14/01/2020 13:37, Jonathan Wakely wrote:

>> I really think people should be reviewing what they're about to push

>> before doing it.

>>

>> OK for wwwdocs?

>>

>

>I'd recommend

>

>git push origin HEAD:<target-branch>

>

>rather than just 'git push'

>

>Otherwise, OK.


Here's a different proposal, which adds your recommendation (in
addition to the simple 'git push') and also tries to address Richi's
concern by simplifying the instructions for checking what you're about
to push.
commit 067f0438c20e740abe71979ff82d5a5ec5bef484
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jan 14 14:35:46 2020 +0000

    Recommend reviewing local changes before pushing them

diff --git a/htdocs/gitwrite.html b/htdocs/gitwrite.html
index 85a0da2d..3b8b18dd 100644
--- a/htdocs/gitwrite.html
+++ b/htdocs/gitwrite.html
@@ -274,7 +274,14 @@ made after you called <code>git add</code> for them.</strong></li>
 <li>After exiting the editor, Git will check in your
 changes <strong>locally</strong>.  When your prompt returns
 the <strong>local</strong> checkin is finished, but you still need to
-push the changes to the shared repository; do <code>git push</code>.
+push the changes to the shared repository.  Before pushing it to gcc.gnu.org,
+make sure your local branch only contains the changes you expect,
+e.g. by reviewing the <code>git status</code> or <code>git log</code> output.
+Once you're ready to publish the changes, do <code>git push</code>,
+or to be more explicit,
+<code>git push origin HEAD:<i>&lt;target branch&gt;</i></code>,
+where <code><i>&lt;target branch&gt;</i></code> is the branch on the remote,
+such as <code>master</code> or <code>releases/gcc-9</code>.
 A message will be sent to the gcc-cvs mailing list indicating that a
 change was made.  If <code>git push</code> gives an error because
 someone else has pushed their own changes to the same branch,
Richard Biener Jan. 14, 2020, 2:47 p.m. | #5
On Tue, Jan 14, 2020 at 3:28 PM Jonathan Wakely <jwakely@redhat.com> wrote:
>

> On 14/01/20 15:08 +0100, Richard Biener wrote:

> >On Tue, Jan 14, 2020 at 2:37 PM Jonathan Wakely <jwakely@redhat.com> wrote:

> >>

> >> I really think people should be reviewing what they're about to push

> >> before doing it.

> >>

> >> OK for wwwdocs?

> >

> >So I figure that first doing the git push with -n -v and then reviewing

> >the pushed changes (cut&paste the revision range) with git show

> >is a workflow easy to remember and less "implicit" than your wording

>

> "cut&paste the revision range" seems awfully tedious to me ;-)

>

> The equivalent is 'git show @{u}..' or if you don't want to remember

> the @{u} thing (aka @{upstream}), you can name the branch explicitly:

>

>    git show origin/master..

>

> or even more explicitly (but identical):

>

>    git show origin/master..HEAD

>

> >(I dont' understand what I actually should _do_ with to review the

> >changes for your description)

>

> If you run the commands, isn't it obvious? (Genuine question)

>

> master wwwdocs$ git log @{u}..

> commit 0eecf8a69a52cd27faeea64c8269a6df3f8db855 (HEAD -> master)

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:34:02 2020 +0000

>

>      Recommend reviewing local changes before pushing them

>

> commit a449e14116456cf66ea15d4aca203a50b55f6e20

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:16:12 2020 +0000

>

>      Fix indentation of .ssh/config snippet

>

> Here git log shows two local commits. If I push, they'll both be

> pushed. Is that what I meant to do, or did I forget that a449e1411 was

> also in my local branch?

>

> You should check that only the commits you expect are shown in the

> log.

>

> You can also just do "git log" and check which commits are more recent

> than the one shown as the upstream:

>

> $ git log

> commit 9e502f6deae9f821bd7079aad5f98a4f3bae15cf (HEAD -> master)

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:34:02 2020 +0000

>

>      Recommend reviewing local changes before pushing them

>

> commit 83f833b74f2dada4235f1b68b2e3cab5e5bba757

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:16:12 2020 +0000

>

>      Fix indentation of .ssh/config snippet

>

> commit 10463a79371068a0b32d8babefb9cf2ee409f4d1 (origin/master, origin/HEAD)

> Author: Christophe Lyon <christophe.lyon@linaro.org>

> Date:   Tue Jan 14 13:48:19 2020 +0000

>

>      [arm] Document -mpure-code support for v6m in gcc-10

>

> The one marked (origin/master, origin/HEAD) is the commit that is

> upstream, the ones before it are local.

>

> Maybe I should just say use 'git status' and/or 'git log' and check

> that your local branch doesn't contain anything unexpected.

>

> The other command shows the diff between your local branch and

> upstream. Check the diff only contains the differences you expect it

> to.


Guess the -n -v approach is more obvious that it shows what a
random git push command will do rather your description which
is vague as to what kind of push it applies to?

Richard.
Andreas Schwab Jan. 14, 2020, 3 p.m. | #6
On Jan 14 2020, Jonathan Wakely wrote:

> You can also just do "git log" and check which commits are more recent

> than the one shown as the upstream:

>

> $ git log

> commit 9e502f6deae9f821bd7079aad5f98a4f3bae15cf (HEAD -> master)

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:34:02 2020 +0000

>

>     Recommend reviewing local changes before pushing them

>

> commit 83f833b74f2dada4235f1b68b2e3cab5e5bba757

> Author: Jonathan Wakely <jwakely@redhat.com>

> Date:   Tue Jan 14 13:16:12 2020 +0000

>

>     Fix indentation of .ssh/config snippet

>

> commit 10463a79371068a0b32d8babefb9cf2ee409f4d1 (origin/master, origin/HEAD)

> Author: Christophe Lyon <christophe.lyon@linaro.org>

> Date:   Tue Jan 14 13:48:19 2020 +0000

>

>     [arm] Document -mpure-code support for v6m in gcc-10

>

> The one marked (origin/master, origin/HEAD) is the commit that is

> upstream, the ones before it are local.


Note that you need have log.decorate enabled in your config, or use git
log --decorate to enable the markers.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Jonathan Wakely Jan. 14, 2020, 3:26 p.m. | #7
On 14/01/20 16:00 +0100, Andreas Schwab wrote:
>On Jan 14 2020, Jonathan Wakely wrote:

>

>> You can also just do "git log" and check which commits are more recent

>> than the one shown as the upstream:

>>

>> $ git log

>> commit 9e502f6deae9f821bd7079aad5f98a4f3bae15cf (HEAD -> master)

>> Author: Jonathan Wakely <jwakely@redhat.com>

>> Date:   Tue Jan 14 13:34:02 2020 +0000

>>

>>     Recommend reviewing local changes before pushing them

>>

>> commit 83f833b74f2dada4235f1b68b2e3cab5e5bba757

>> Author: Jonathan Wakely <jwakely@redhat.com>

>> Date:   Tue Jan 14 13:16:12 2020 +0000

>>

>>     Fix indentation of .ssh/config snippet

>>

>> commit 10463a79371068a0b32d8babefb9cf2ee409f4d1 (origin/master, origin/HEAD)

>> Author: Christophe Lyon <christophe.lyon@linaro.org>

>> Date:   Tue Jan 14 13:48:19 2020 +0000

>>

>>     [arm] Document -mpure-code support for v6m in gcc-10

>>

>> The one marked (origin/master, origin/HEAD) is the commit that is

>> upstream, the ones before it are local.

>

>Note that you need have log.decorate enabled in your config, or use git

>log --decorate to enable the markers.


Thanks. Looks like log.decorate=auto is the default (so they're shown
when the log goes to a terminal, not otherwise).

Patch

diff --git a/htdocs/gitwrite.html b/htdocs/gitwrite.html
index 85a0da2d..98ac7201 100644
--- a/htdocs/gitwrite.html
+++ b/htdocs/gitwrite.html
@@ -274,7 +274,13 @@  made after you called <code>git add</code> for them.</strong></li>
 <li>After exiting the editor, Git will check in your
 changes <strong>locally</strong>.  When your prompt returns
 the <strong>local</strong> checkin is finished, but you still need to
-push the changes to the shared repository; do <code>git push</code>.
+push the changes to the shared repository. Before pushing it to gcc.gnu.org,
+review your local changes to make sure your local branch only contains
+the changes you expect, e.g. <code>git log @{u}..</code> or
+<code>git diff @{u}</code> (where <code>@{u}</code> refers to the upstream
+branch, most likely something like <code>origin/master</code> or
+<code>origin/releases/gcc-9</code>).
+Once you're ready to publish the changes, do <code>git push</code>.
 A message will be sent to the gcc-cvs mailing list indicating that a
 change was made.  If <code>git push</code> gives an error because
 someone else has pushed their own changes to the same branch,