Add Git Bootcamp page #144

Mariatta · 2017-03-13T06:48:58Z

The no frills guide to git.

Closes #125, python/core-workflow#27

orsenthil

Contents LGTM. For the page itself, suggest getting a sign-off from Brett.

willingc

This is wonderful! 🍰 🍪

It's great as is. Do you want to add "how to merge a PR into master" too?

brettcannon · 2017-03-13T17:06:33Z

gitbootcamp.rst

+
+   $ git checkout master
+   $ git fetch upstream
+   $ git rebase upstream/master


Why do you have to rebase the master branch if you haven't committed to it? Is this a step that git pull does for you?

Difference in workflow approach. Some (myself included) use rebase/fetch instead of git pull.

Why is that? What's the pros/cons of the two approaches? (Basically why do you like the extra steps since this is meant for people not familiar with git and thus we might want to prefer the fewer-steps approach?)

git pull --rebase would be similar in approach and one could configure git pull to use rebase by default instead of merge.

I've only ever used git pull upstream master in any other projects I've used. To me it says more clearly what it's doing - "pull what's new on upstream onto my master branch". AFAIK, that's the normal advice (I don't recall where I learned the workflow from, but likely from various tutorials online). I think we should stick to the "normal" approach, as best as we can determine it, so we don't get newcomers being confused as to why we do things differently on Python.

I'd assume that fetch/rebase is intended more for cases where you're making changes on your local master branch. But my normal workflow is to only ever have master as a pristine copy of upstream's master, so pull upstream master says what I'd expect.

OK, cool. I wasn't sure if this was intended as a recommendation, is all - to me, it's more confusing than a simple git pull, and I'd tend to go for the simpler suggestion by default. But if we're saying the same thing, that's fine :-)

I think @pfmoore picked up on my initial confusion. As the instructions are just to keep master in sync then the rebase step seemed odd to me since you're not rebasing any previous commits you made locally. Maybe a comment at the start saying "there are alternative ways to perform the synchronization, but the approach taken to provide steps that are the same across scenarios"?

That's why I suggest git pull --rebase upstream master.

I think @zware's suggestion is the best in terms of simplicity and flexibility.

I'd be inclined to prefer @zware's git pull --rebase upstream master, with a comment "you only need --rebase if you have local changes to the branch", That's simple enough for newcomers, explains when --rebase is needed, and remains consistent across scenarios.

brettcannon · 2017-03-13T17:07:12Z

gitbootcamp.rst

+
+Go to https://github.com/python/cpython to create the pull request.  Select
+``3.6`` as the base branch, and ``backport-someissue-3.6`` as the head branch.
+


You forgot to mention your script. 😄

Once it's moved to core-workflow, I'll update this :)

brettcannon · 2017-03-13T17:07:48Z

LGTM, but honestly I think having Ezio look over this would be great since I know he has asked for more of a git intro in the devguide.

zware · 2017-03-13T18:01:21Z

gitbootcamp.rst

+      $ cd UserX
+      $ git clone https://github.com/UserX/cpython
+      $ cd cpython
+      $ git checkout fixspamlib


This can be simplified significantly. GitHub offers pull requests as remote branches that are not pulled automatically, but you can pull them explicitly by doing git fetch upstream pull/${1}/head:pr/${1} && git checkout pr/${1} where ${1} is the PR number. @berkerpeksag has this nicely packaged up as an alias:

[alias] pr = !sh -c \"git fetch upstream pull/${1}/head:pr/${1} && git checkout pr/${1}\" -

which allows one to do git pr 615 to fetch and check out python/cpython#615. The local branch name (pr/${1} or pr/615 here) can be adjusted as you see fit; I might prefer pr_${1}.

Very interesting.. I didn't know about this :)

As an FYI (though we don't need to document), hub makes a maintainer's job easier (similar to @zware's mention of @berkerpeksag's alias).

- explain how to set up `git pr` alias - explain how to accept and merge a PR - provide an example of good squash commit message

Mariatta · 2017-03-14T05:46:00Z

Made some adjustments:

use the git pull --rebase upstream master approach
explain how to create git pr alias
explain how to merge and squash a PR
provide example of good commit message

brettcannon · 2017-03-14T16:45:03Z

gitbootcamp.rst

+
+From your command line::
+
+   $ git clone [email protected]:UserName/cpython.git


Might want to add a note about substituting UserName or use something like <username> in case UserName on GitHub actually has CPython forked. 😉

brettcannon · 2017-03-14T16:45:55Z

gitbootcamp.rst

+
+   $ git checkout -b some-branch master # creates a new branch off master
+
+which is equal to::


Might want to mention it's equivalent if you are starting from the master branch, otherwise explicitly add the master part to the git branch step.

Thanks, I made the change in line 56.

brettcannon · 2017-03-14T16:48:36Z

gitbootcamp.rst

+   $ git pr 777
+
+
+Accepting and Merging Pull Request


"Accepting and Merging a Pull Request"

brettcannon · 2017-03-14T16:48:55Z

gitbootcamp.rst

+
+3. Select the base fork: ``python/cpython`` and base branch: ``master``
+
+4. Select the head fork: ``UserName/cpython`` and base branch: the branch


<username> for UserName

- change UserName into <username> - be explicit about branching off master

brettcannon · 2017-03-14T19:45:01Z

gitbootcamp.rst


 Create a new branch::

   $ git checkout -b some-branch master # creates a new branch off master

 which is equal to::

-   $ git branch some-branch # create 'some-branch', without checking it out
+   $ git branch some-branch master # create 'some-branch' off 'master, without checking it out


I think you're missing an apostrophe at the end of "master".

Right 😅 Thanks. Fixed.

ezio-melotti

Even though I left lot of comments, mostly are minor issues, and overall I like the page.
This however adds yet another page, so in the future this might end up replacing or being merged with the other git-related pages.

ezio-melotti · 2017-03-14T20:36:54Z

gitbootcamp.rst

+Git Bootcamp and Cheat Sheet
+============================
+
+In this section, we'll go over some commonly used git commands that are


Just a nit: I think this should either be "Git" or :cmd:`git`.

ezio-melotti · 2017-03-14T20:38:11Z

gitbootcamp.rst

+
+1. Go to https://github.com/python/cpython.
+
+2. Press ``Fork``.


"Press the Fork button on the top-right".

ezio-melotti · 2017-03-14T20:38:45Z

gitbootcamp.rst

+
+3. When asked where to fork the repository, choose to fork it to your username.
+
+4. A fork will be created at https://github.com/<username>/cpython.


ezio-melotti · 2017-03-14T20:39:45Z

gitbootcamp.rst

+Cloning The Forked CPython Repository
+-------------------------------------
+
+From your command line::


In this section you could also mention that this is only necessary once.

ezio-melotti · 2017-03-14T20:43:18Z

gitbootcamp.rst

+
+   $ git clone [email protected]:<username>/cpython.git
+   $ cd cpython
+   $ git remote add upstream [email protected]:python/cpython.git


Using $ has the disadvantage that is not possible to copy/paste all the instructions at once, but it makes clear that these are shell commands. Since most of the commands are one-liners, it might be ok to leave the $.

ezio-melotti · 2017-03-14T22:54:35Z

gitbootcamp.rst

+
+For example, to fetch and checkout pull request #777::
+
+   $ git pr 777


I think this is the only example in the page -- elsewhere you only used .
I'm not sure it's needed -- it doesn't hurt having it, but it's not particularly useful either, and it's a bit inconsistent with the rest.

I'll remove the example to make it consistent.

ezio-melotti · 2017-03-14T23:03:00Z

gitbootcamp.rst

+For example, to fetch and checkout pull request #777::
+
+   $ git pr 777
+


Another scenario that is not covered here is: how can I update a PR that I pulled from a contributor (e.g. to add Misc/NEWS)?
This came up on #python-dev a few days ago, and afaiu:

you can add your changes and push to the contributor repo using git push -u [email protected]:<contributor>/cpython.git <branch> (or add a remote with git remote add <contributor> [email protected]:<contributor>/cpython.git and then git push -u <contributor> <branch>). In order to do this the contributor must have allowed the maintainers to edit the prs (should be allowed by default).

you can add your changes, push the branch to your fork, and create a new and separate PR that replaces the original one.

It was not clear how the author is determined if multiple users (contributor and core-dev) worked on the same PR: author of the first commit? owner of the repo the PR comes from? author with most commits/loc in the PR?

I haven't had to do this myself, so I don't have the answer ...

ezio-melotti · 2017-03-14T23:09:14Z

gitbootcamp.rst

+If the change has been committed, and now you want to reset it to whatever
+the origin is at::
+
+   $ git reset --hard HEAD


There are two more scenarios where I used hg revert that are not covered here:

how to revert all changes in the working copy (i.e. the equivalent of hg revert -a -- git reset? git reset --hard?).

how to revert the fix while leaving the test to verify if they fail without the fix.

The second case works differently since we are using PRs. With HG I could apply a patch and do hg revert Lib/ to revert all the changes in Lib while leaving the tests unchanged. I think the git equivalent would be something like git checkout master Lib/, after I pulled a PR as described in a later section.

ezio-melotti · 2017-03-14T23:14:06Z

gitbootcamp.rst

+Accepting and Merging A Pull Request
+------------------------------------
+
+Pull requests can be accepted and merged by a Python Core Developer.


A question about "merging etiquette": if another core-dev submitted a PR, and it looks good to me, should I go ahead and merge it, or should I just leave a positive review and let the original core-dev merge it?
I think the latter is better, but I've seen the former happen.
Perhaps this could be clarified here.

Perhaps this is to be asked at core-workflow or python-committers? :)

ezio-melotti · 2017-03-14T23:14:27Z

gitbootcamp.rst

+
+
+Accepting and Merging A Pull Request
+------------------------------------


This should be marked as core-devs only, since contributors can't accept/merge.

Mentioned in the next line.

willingc · 2017-03-15T01:46:39Z

@ezio-melotti Wouldn't it make more sense since we are now on GitHub to merge this PR as is and then create a new PR for any other changes you wish to make?

ezio-melotti · 2017-03-15T01:50:47Z

That would be fine with me, since there are no major problems with the patch.
I'll change my review to "approved" as not to block this, but let @Mariatta decide whether she wants to address the comments in this PR, create a separate PR, or leave it to me (in that case I still need some feedback for the comments I made).

willingc · 2017-03-15T01:53:21Z

Thanks @ezio-melotti. Your flexibility is appreciated. I suspect this will get iterated over time as well. 👍

Mariatta · 2017-03-15T19:41:17Z

Thanks for reviewing, @ezio-melotti 😃
My intention with this page is not to provide a full tutorial of git, but to provide a quick reference page.
For example, when a contributor forgot the command of how to rebase, they can go to this page and quickly find the command they need.
I also want this page to be useful at sprints where the goal is just to know enough git commands to start contributing to the real thing 😉
Because of this, I think this page should be kept brief and concise, and not too wordy.
There are various ways one can work with git to achieve the same results. Individuals can still choose to work with a workflow that they're comfortable with.

I will prepare another update to this PR later today to address some of your feedback.

There are definitely more topics that need to be covered here, but we can address them in future PRs :)
for example

how to resolve merge conflict
git log
git show

Thanks :)

- replace current instruction to backport with a reference to cherry_picker.py - add ..highlight:: console - some formatting

ezio-melotti · 2017-03-16T05:52:29Z

The latest changeset LGTM.

Mariatta · 2017-03-16T06:02:12Z

Thanks for all the feedback and for reviewing 🙋 💯 🎉

Provide a list of useful Git commands that are relevant to CPython's workflow, such as: - forking and cloning the repo - branching - committing - backporting - creating pull requests

Add Git Bootcamp page

142f1df

the-knights-who-say-ni added the CLA signed label Mar 13, 2017

orsenthil approved these changes Mar 13, 2017

View reviewed changes

willingc approved these changes Mar 13, 2017

View reviewed changes

brettcannon reviewed Mar 13, 2017

View reviewed changes

brettcannon requested a review from ezio-melotti March 13, 2017 17:07

zware reviewed Mar 13, 2017

View reviewed changes

- use the git pull --rebase upstream master command

4321772

- explain how to set up `git pr` alias - explain how to accept and merge a PR - provide an example of good squash commit message

willingc approved these changes Mar 14, 2017

View reviewed changes

brettcannon reviewed Mar 14, 2017

View reviewed changes

More updates

2104dea

- change UserName into <username> - be explicit about branching off master

brettcannon reviewed Mar 14, 2017

View reviewed changes

add missing apostrophe

efee4f7

brettcannon approved these changes Mar 14, 2017

View reviewed changes

Change SomeUser into CoreDeveloper

2993cbc

ezio-melotti requested changes Mar 14, 2017

View reviewed changes

ezio-melotti approved these changes Mar 15, 2017

View reviewed changes

Some improvements

6284566

- replace current instruction to backport with a reference to cherry_picker.py - add ..highlight:: console - some formatting

Mariatta merged commit a57fa8b into python:master Mar 16, 2017

Mariatta deleted the git-bootcamp branch March 16, 2017 06:00


		Go to https://github.com/python/cpython to create the pull request. Select
		``3.6`` as the base branch, and ``backport-someissue-3.6`` as the head branch.


		From your command line::

		$ git clone [email protected]:UserName/cpython.git


		$ git checkout -b some-branch master # creates a new branch off master

		which is equal to::


		3. Select the base fork: ``python/cpython`` and base branch: ``master``

		4. Select the head fork: ``UserName/cpython`` and base branch: the branch


		1. Go to https://github.com/python/cpython.

		2. Press ``Fork``.


		3. When asked where to fork the repository, choose to fork it to your username.

		4. A fork will be created at https://github.com/<username>/cpython.


		For example, to fetch and checkout pull request #777::

		$ git pr 777



		Accepting and Merging A Pull Request
		------------------------------------

Uh oh!

Add Git Bootcamp page #144

Add Git Bootcamp page #144

Uh oh!

Conversation

Mariatta commented Mar 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orsenthil left a comment

Choose a reason for hiding this comment

Uh oh!

willingc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettcannon commented Mar 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mariatta commented Mar 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezio-melotti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Mariatta commented Mar 13, 2017 •

edited

Loading