-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Move the "quick reference" to a dedicated page #1838
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.
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
Changes from 6 commits
9a62b60
543e002
fec3bf3
cdc0df0
5e7e09e
86f942c
1a76f5e
09e61f7
a908456
1931332
608a224
85825ba
7f14dbc
e54a2bb
0265531
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ Getting started | |
| .. toctree:: | ||
| :maxdepth: 5 | ||
|
|
||
| quick-reference | ||
| setup-building | ||
| fixing-issues | ||
| git-boot-camp | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| .. _quick-reference: | ||
|
|
||
| =============== | ||
| Quick reference | ||
| =============== | ||
|
sirosen marked this conversation as resolved.
|
||
|
|
||
| Here are the basic steps needed to get set up and open a pull request. | ||
|
|
||
| This is meant as a checklist and cheat-sheet, not a comprehensive guide. | ||
| For complete instructions please see the :ref:`setup guide <setup>` and the | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
| :ref:`pull request guide <pullrequest>`. | ||
|
|
||
|
|
||
| Setup Git | ||
| ========= | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| Install and set up ``Git``. | ||
|
|
||
| For detailed setup information, see :ref:`"Install Git" <vcsetup>`. | ||
| There is also a more detailed :ref:`Git guide and cheat sheet <gitbootcamp>`. | ||
|
|
||
| Fork and clone the repo | ||
| ----------------------- | ||
|
|
||
| Fork `the CPython repository <https://github.com/python/cpython>`__ | ||
| to your GitHub account and :ref:`get the source code <checkout>` using:: | ||
|
|
||
| git clone https://github.com/<your_username>/cpython | ||
| cd cpython | ||
|
|
||
| We recommend also setting up ``pre-commit``:: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @encukou, are you happy to do this now that we've hash-pinned all the third party hooks?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the policy on updating those hash-pins? (FWIW, you don't need to make me happy. Just Seth ;)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None yet. I'd like to automate with either:
And we can manually update when we need a new feature or bugfix.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I take it that there was some discussion about ensuring these are pinned for safety. 😁 I use pre-commit.ci for this in most of my projects, and the autofixing is quite nice. If the autofixing isn't desirable, I'd tend towards Dependabot, since the infrastructure is provided by GitHub. (I think this is nicer in having fewer providers and putting less load on a smaller platform.)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, this doesn't spark joy here. It don't want to recommend this setup. But I won't block it, if it's not mandatory. From
I see this differently. Developers spend a fair chunk of time looking at code while their fingers are engaged. That said, I agree that there are automatic checks that are useful -- unused
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have similar feelings as @encukou about autofix (though I do have it enabled on CI in other projects. In general, I would have CI run the pre-commit run. Locally, I think it should be optional, not recommended (a subtle difference). Folks, I think this is the only item that needs a decision after cleanup of a few commands where there is consensus. Let's try to converge so that we can merge. Thanks!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps I've missed something, but it seems that the original page didn't mention pre-commit, and this is an addition. Since the goal of this PR is to move content, and this point is becoming difficult, shouldn't we let the status quo win and not add pre-commit now?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this was something I added, in the course of walking through the original and cross-checking it against the full guide. I hadn't expected it to be contentious, as it's documented as "recommended" in the more complete docs. I would be okay with changing that to "optional" here. That said, I prefer dropping it. Doing so makes the guide shorter, and this is meant to be a "quick reference". I'll plan to do that when I come back to apply (final? 🤞 ) changes, probably in a few hours time.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's drop it. We can always add it in a future PR. |
||
|
|
||
| pre-commit install | ||
|
|
||
| For detailed information, see :ref:`"Get the source code" <checkout>` and | ||
| :ref:`"Install pre-commit as a Git hook" <install-pre-commit>`. | ||
|
|
||
|
|
||
| Build Python | ||
| ============ | ||
|
|
||
| .. tab:: Unix | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| ./configure --config-cache --with-pydebug && make -j $(nproc) | ||
|
|
||
| .. tab:: macOS | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| ./configure --config-cache --with-pydebug && make -j8 | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| .. tab:: Windows | ||
|
|
||
| .. code-block:: dosbatch | ||
|
|
||
| PCbuild\build.bat -e -d | ||
|
|
||
| See also :ref:`more detailed instructions <compiling>`, | ||
| :ref:`how to install and build dependencies <build-dependencies>`, | ||
| and the platform-specific pages for :ref:`Unix <unix-compiling>`, | ||
| :ref:`macOS <macOS>`, and :ref:`Windows <windows-compiling>`. | ||
|
|
||
|
|
||
| Run the tests | ||
| ============= | ||
|
|
||
| .. tab:: Unix | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| ./python -m test -j3 | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| .. tab:: macOS | ||
|
|
||
| .. code-block:: shell | ||
|
|
||
| ./python.exe -m test -j8 | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| .. note:: | ||
| :ref:`Most <mac-python.exe>` macOS systems use | ||
| :file:`./python.exe` in order to avoid filename conflicts with | ||
| the ``Python`` directory. | ||
|
StanFromIreland marked this conversation as resolved.
|
||
|
|
||
| .. tab:: Windows | ||
|
|
||
| .. code-block:: dosbatch | ||
|
|
||
| .\python.bat -m test -j3 | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
|
|
||
| See also :ref:`how to write and run tests <run-write-tests>`. | ||
|
|
||
|
|
||
| .. _pullrequest-quickguide: | ||
|
|
||
| Create issues and pull requests | ||
| =============================== | ||
|
|
||
| Create issues for nontrivial changes | ||
| ------------------------------------ | ||
|
|
||
| For most changes, `create an issue <https://github.com/python/cpython/issues>`__ | ||
| before submitting a pull request. | ||
| Trivial changes like typo fixes do not need issues. | ||
|
|
||
| Create work branches | ||
| -------------------- | ||
|
|
||
| Work on a feature or fix in a new branch in Git from the ``main`` branch:: | ||
|
|
||
| git checkout -b fix-issue-12345 main | ||
|
|
||
| Make changes, then :ref:`commit <commit-changes>` and | ||
| :ref:`push to your fork <push-changes>`. | ||
|
|
||
| Document your changes | ||
| --------------------- | ||
|
|
||
| Many changes deserve a NEWS entry which documents what changed. | ||
|
|
||
| Add a News entry into the ``Misc/NEWS.d/`` directory as individual file. | ||
| The news entry can be created by using | ||
| `blurb-it <https://blurb-it.herokuapp.com/>`__, | ||
| or the :pypi:`blurb` tool and its ``blurb add`` command. | ||
|
|
||
| .. tip:: | ||
|
|
||
| You can read more about ``blurb`` in its | ||
| `repository <https://github.com/python/blurb>`__. | ||
|
|
||
| For more information on writing news entries, | ||
| see :ref:`"Updating NEWS and What's New in Python" <news-entry>`. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be higher, we don't want unnecessary news entries (it explains when you should(n't) add one).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps move this to line 124. Then, start a new paragraph for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to keep a link to the detailed reference at the end of this subsection if we can manage it. Keeping the ordering of the content consistent section to section makes the doc more navigable. I can definitely edit the perhaps-too-pithy "many changes deserve a NEWS entry" up above.
If I reproduce this whole list, the quick-reference begins to wander into being a not-so-quick-reference. Would this update be satisfactory? -Many changes deserve a NEWS entry which documents what changed.
+Most user-visible changes, other than documentation changes,
+deserve a NEWS entry which documents what changed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concurrent comments! So the possibility with that approach is something like...
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this suggestion @sirosen. Thoughts @StanFromIreland?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems good, but we can drop the Additionally, I'd reword the second paragraph a little, like so: A news entry can be created locally with the :pypi:`blurb` tool
and its ``blurb add`` command or online after a pull request has
been opened with blurb-it <https://blurb-it.herokuapp.com/>__.We've generally discouraged manual creation as it complicates things for new contributors. Users who want to do so can see the instructions in the How to add a news entry section. |
||
|
|
||
| Create pull requests | ||
| -------------------- | ||
|
|
||
| Create pull bequests on GitHub from your branches, on your fork, and make sure | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
| to put the relevant issue number in ``gh-NNNNNN``` format in the pull request title. | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
| For example: | ||
|
|
||
| .. code-block:: text | ||
|
|
||
| gh-12345: Fix some bug in spam module | ||
|
|
||
| See also, GitHub's documentation on `creating Pull Requests`_. | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| For more detailed guidance, follow the :ref:`step-by-step pull request guide <pullrequest-steps>`. | ||
|
|
||
| .. note:: | ||
|
|
||
| First time contributors will need to sign the Contributor Licensing | ||
| Agreement (CLA) as described in the :ref:`Licensing <cla>` section of | ||
| this guide. | ||
|
|
||
| .. _creating Pull Requests: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| Work on your pull request | ||
| ------------------------- | ||
|
|
||
| Make sure the | ||
| :ref:`continuous integration checks on your Pull Request are green <keeping-ci-green>` (successful). | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| Read and respond to reviewer comments on your pull request. | ||
|
|
||
| See also, GitHub's documentation on `commenting on Pull Requests`_. | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
|
|
||
| .. note:: | ||
|
|
||
| In order to keep the commit history intact, please avoid squashing or amending | ||
|
sirosen marked this conversation as resolved.
Outdated
|
||
| history and then force-pushing to the PR. | ||
| Reviewers often want to look at individual commits. | ||
|
|
||
| CPython uses squash merges, so PRs will end up as single commits when merged. | ||
|
|
||
| .. _commenting on Pull Requests: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request | ||
Uh oh!
There was an error while loading. Please reload this page.