Skip to content

adding pullrequest guide for #103 #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mariatta merged 19 commits into from
Apr 12, 2017
Merged

adding pullrequest guide for #103 #140

Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9e3cc06
adding pullrequest guide for #103
louisrawlins Mar 5, 2017
b62d78d
replaces GitHub, word caps
louisrawlins Mar 11, 2017
21b4960
edits - create issue
louisrawlins Mar 11, 2017
0b68c42
edits - upstream remote
louisrawlins Mar 11, 2017
523b77f
clearer create issue
louisrawlins Mar 11, 2017
e0aac2a
edits - cpython pull req
louisrawlins Mar 11, 2017
24149d3
edit - pull req
louisrawlins Mar 11, 2017
044dc08
edit - run tests, then commit
louisrawlins Mar 11, 2017
ca8e9c1
https or ssh
louisrawlins Mar 11, 2017
a3ddf1c
clarifying when to celebrate a pr
louisrawlins Mar 11, 2017
a6b773c
adding refs
louisrawlins Mar 11, 2017
ab07aef
using internal refs when available"
louisrawlins Mar 11, 2017
cebd349
swaping feature for branch
louisrawlins Mar 11, 2017
2b468f7
clarifying squishing and prs
louisrawlins Mar 11, 2017
36eb29b
alt, submit patch to bpo
louisrawlins Mar 11, 2017
1ae31e5
added ssh https ref
louisrawlins Mar 11, 2017
9865ccf
review adds to guide
louisrawlins Mar 23, 2017
2a7bb10
proper rst refs
louisrawlins Mar 24, 2017
a37d2e6
Add a missing word
Mariatta Apr 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
using internal refs when available"
  • Loading branch information
@louisrawlins
louisrawlins committed Mar 11, 2017
commit ab07aefacd8fe48be55c2bbf40a67c4899be9973
36 changes: 15 additions & 21 deletions pullrequest.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ command (after any successful build of Python)::
python.bat Tools/scripts/patchcheck.py


.. _pullrequest-quickguide:

Quick Guide
'''''''''''

Expand All @@ -103,55 +105,47 @@ Here is a quick overview of you can contribute to CPython on GitHub:

1. If an Issue doesn't exist, `create an Issue`_ that describes your change

2. `Get started`_ and set up your system
2. Prepare your system by :ref:`setup`

3. Fork `CPython`_ on GitHub (using the Fork button in the upper-right on GitHub)

4. `Compile Python`_ on your system

5. `Run tests`_ after you have built Python
4. Start :ref:`compiling` to build Python

6. `Add an "upstream" Git remote`_ on your system (for SSH, or you can `use HTTPS`_)
5. Begin :ref:`runtests` after you have built Python

7. `Create a Branch in Git`_ where you can work on changes
6. Set your Git :ref:`remote-configuration` to add an "upstream" remote (using SSH, or you can `use HTTPS`_)

8. `Run tests`_ again
7. Create :ref:`committing-active-branches` in Git where you can work on changes

@Mariatta Mariatta Mar 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reference to committing-active-branches is incorrect here.
Currently it links to the section that explains how to list the branches (git branch command.)
It's better to refer to line 153 below (about git checkout -b BRANCH_NAME) which is the key step.

@louisrawlins louisrawlins Mar 12, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Mariatta . I agree. That feels clearer. I was struggling to move back-and-forth between comitting.rst -- some of the naming and explanation didn't fit.


9. Commit changes and `resolve conflicts in Git`_
8. Continue :ref:`runtests`

10. `Push commits`_ to your GitHub repo
9. Prepare for :ref:`committing-push-changes` to your GitHub repo

11. `Create Pull Request`_ for your changes to be reviewed then merged into `CPython`_
10. `Create Pull Request`_ for your changes to be reviewed then merged into `CPython`_

12. Review and address `comments on your Pull Request`_
11. Review and address `comments on your Pull Request`_

13. When your changes are merged, celebrate contributing to Python! :)
12. When your changes are merged, celebrate contributing to Python! :)

.. _Clear communication: https://opensource.guide/how-to-contribute/#how-to-submit-a-contribution
.. _Open Source: https://opensource.guide/
.. _create an Issue: https://bugs.python.org/
.. _Get started: https://cpython-devguide.readthedocs.io/setup.html#getting-started
.. _CPython: https://github.com/python/cpython
.. _Compile Python: https://cpython-devguide.readthedocs.io/setup.html#compiling-for-debugging
.. _Run tests: https://cpython-devguide.readthedocs.io/runtests.html#running-writing-tests
.. _Add an "upstream" Git remote: https://cpython-devguide.readthedocs.io/committing.html#remotes-setup
.. _use HTTPS: https://help.github.com/articles/which-remote-url-should-i-use/
.. _Create a Branch in Git: https://cpython-devguide.readthedocs.io/committing.html#active-branches
.. _resolve conflicts in Git: https://cpython-devguide.readthedocs.io/committing.html#squashing-commits
.. _Run tests: https://cpython-devguide.readthedocs.io/runtests.html#running-writing-tests
.. _Push commits: https://cpython-devguide.readthedocs.io/committing.html#pushing-changes
.. _Create Pull Request: https://help.github.com/articles/creating-a-pull-request/
.. _comments on your Pull Request: https://help.github.com/articles/commenting-on-a-pull-request/


.. _pullrequest-steps:

Quick Guide Step-by-step
''''''''''''''''''''''''

Set up your system::

@willingc willingc Mar 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only work if the user has an ssh key setup for GitHub. Otherwise, the user will need to use https.

@louisrawlins louisrawlins Mar 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm re-reading this part. How often to contributors not have an SSH / RSA key? To me, that seems standard.

@willingc willingc Mar 11, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@louisrawlins It's worth mentioning the SSH key since my experience at sprints and hackathons is that many new contributors do not have an SSH key configured for GitHub.

@louisrawlins louisrawlins Mar 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha. Gotcha. Hadn't considered hackathons and that environment.


git clone git@github.com:YOUR_GITHUB_ID/cpython.git

Replace **YOUR_GITHUB_ID** with your GitHub account name above, then add upstream::
Replace **YOUR_GITHUB_ID** with your GitHub account name above, then add upstream (using SSH, or you can `use HTTPS`_)::

git remote add upstream git://github.com/python/cpython.git

@willingc willingc Mar 5, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be the https url since a contributor would need core dev privileges, as well as an ssh key, to use the git url.

@ezio-melotti ezio-melotti Mar 5, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but I would also add a short note to explain the difference, otherwise users might be wondering why two different types of urls are used.

@louisrawlins louisrawlins Mar 5, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I'm struggling to figure out a way to say that concisely. It's like, you're cloning SSH, then fetching HTTPS -- I suppose?

@ezio-melotti ezio-melotti Mar 5, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#130 has a discussion about it that clarifies the differences.

@louisrawlins louisrawlins Mar 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so much curious about the difference, as I'm trying to figure out how to tell potentially contributors when they might need to worry about the difference.

For me, I treat HTTPS as something I use when I just need to download something as a ZIP, I'm not at my machine, etc. It's not something I'd be using if I'm branching something on my repo and preparing for a Pull Request. For that, I'd use SSH. But perhaps I'm missing a nuance here.

@louisrawlins louisrawlins Mar 11, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "Add an “upstream” Git remote on your system (for SSH, or you can use HTTPS)"?


Expand Down