Skip to content
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

Adding Codespaces Documentation #1144

Merged
merged 19 commits into from
Aug 2, 2023
Merged

Conversation

marlenezw
Copy link
Contributor

@marlenezw marlenezw commented Jul 26, 2023

Hi, I'd like to add documentation to show how to use Codespaces for contributing to CPython with the devcontainer files Brett added. I've added the instructions at the bottom of the Setup and Building page. I initially added them as a separate page but thought this might be more appropriate. Feel free to let me know if you think they should live somewhere else.

This is a PR that would solve #1138. I added some context about the discussions and places where some people had asked for docs in the issue. (Also sorry for the other closed PR's still getting used to contributing :))


📚 Documentation preview 📚: https://cpython-devguide--1144.org.readthedocs.build/

@marlenezw
Copy link
Contributor Author

CC @brettcannon 😄

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, a few minor comments :)

getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
marlenezw and others added 8 commits July 26, 2023 12:15
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@marlenezw
Copy link
Contributor Author

Thank you @hugovk! Your suggestions make sense to me. Let me know if you think there's anything else I should change!

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

The actual text content looks pretty good. I've only very briefly tried codespaces, so it would be good for someone more familiar to check this too.

Oh, by the way, we usually limit lines to around 80 characters (see https://devguide.python.org/documentation/style-guide/#use-of-whitespace). Please could you add some line breaks?

getting-started/setup-building.rst Outdated Show resolved Hide resolved
marlenezw and others added 3 commits July 27, 2023 11:30
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
Co-authored-by: Pradyun Gedam <pradyunsg@gmail.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

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

A couple of suggestions, but they are all minor.

getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
getting-started/setup-building.rst Outdated Show resolved Hide resolved
marlenezw and others added 4 commits August 2, 2023 13:02
Co-authored-by: Brett Cannon <brett@python.org>
Co-authored-by: Brett Cannon <brett@python.org>
Co-authored-by: Brett Cannon <brett@python.org>
Co-authored-by: Brett Cannon <brett@python.org>
@hugovk
Copy link
Member

hugovk commented Aug 2, 2023

I think this is ready for merge, just one thing, please could you add some newlines so each line is no longer than around 80 characters?

This will make it easier to read in the editor, and more importantly, new changes will be easier to review in future PRs.

Thanks!

@marlenezw
Copy link
Contributor Author

Hi @hugovk! Yes! I was just working on that and added them in. Sorry for the delay with that. Appreciate the feedback on it!

@marlenezw marlenezw marked this pull request as ready for review August 2, 2023 11:33
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

All good, thank you very much!

@brettcannon brettcannon merged commit dc0be4d into python:main Aug 2, 2023
2 checks passed
@brettcannon
Copy link
Member

Thanks, Marlene, for the PR and everyone for their comments!

@marlenezw
Copy link
Contributor Author

Thank you Brett, Hugo, Erlend and Pradyun for the help with this!!! My first PR to Python 😄 Very excited and happy to help!

@hugovk
Copy link
Member

hugovk commented Aug 2, 2023

Congratulations and good work! This guide should be useful for the PyCon Korea sprint next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants