Update the setup and PR lifecycle pages #265
Conversation
|
@ezio-melotti Is this still WIP or ready for review? Lots of good improvements here 👍 |
|
There are still a few changes I want to make before it's ready -- it shouldn't take too long. |
ezio-melotti
changed the title
|
That's enough commits for this PR :) |
| The automated patch check doesn't actually *answer* all of these | ||
| questions. Aside from the whitespace checks, the tool is | ||
| a memory aid for the various elements that can go into | ||
| making a complete patch. |
There was a problem hiding this comment.
This has been moved to the pullrequest page.
| feature/bugfix. | ||
|
|
||
| It is of course okay to pile up several commits to one branch and merge them | ||
| into another in one commit. |
There was a problem hiding this comment.
This has been moved to pullrequest and combined with the next red chunk.
| contributed to the resolution, it is good practice to credit them. | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
This has been combined with the previous chunk.
| (note that not all checks apply to non-core developers). On Windows, use this | ||
| command (after any successful build of Python):: | ||
|
|
||
| python.bat Tools/scripts/patchcheck.py |
There was a problem hiding this comment.
This has been combined with the other patchcheck section and has been moved further down.
|
Thanks @ezio-melotti It's looking great! |
There was a problem hiding this comment.
@ezio-melotti One small comment as something to think about. I would merge this PR as is and address that in a future PR if needed. Great work! 🎉 🍰
| 6. Configure an ``upstream`` remote:: | ||
|
|
||
| $ cd cpython | ||
| $ git remote add upstream [email protected]:python/cpython.git |
There was a problem hiding this comment.
@ezio-melotti I think this is correct if you have push privileges on the repo, but others likely will need to use https for the upstream repo.
There was a problem hiding this comment.
One has to be using ssh for git@github to work. I believe we agreed to consistently use https: (and git's credential keeper) in the instructions as using both randomly is confusing.
There was a problem hiding this comment.
But for the record, it does work even if you don't have push privileges for the repo, if you are using ssh already with github.
There was a problem hiding this comment.
This was already discussed on #263. The outcome was that by using this URL we don't need to have separate instructions for core devs and contributors. Pulling works for both, and pushing to upstream would fail anyway if contributors don't have push privileges.
This was also moved from the old instructions, I didn't change it :)
|
Thanks everyone for the reviews! |
* Update the setup page and the quick guides. * Working and markup tweaks. * Remove the "Tool Setup" section. * Rename the "Creating" section to "Introduction". * Reorganize info about patchcheck. * Rename and move the "Preparation" section to "Making good PRs". * Fix headers levels (by rotating quotes). * Remove outdated comment. * Move and refactor the sections about commits. * Fix markup and long lines. * Convert the step-by-step guide to a bullet list.
This is part of #120, and it builds on #263. The goal of this PR is to reorganize the setup and pullrequest pages.
The end result will be: