Skip to content

Conversation

ember91
Copy link
Contributor

@ember91 ember91 commented Oct 16, 2024

This includes:

  • .rst
  • .md
  • .txt
  • Misc/HISTORY
  • Tool/freeze/README

See #125473 for context.


📚 Documentation preview 📚: https://cpython-previews--125569.org.readthedocs.build/

@bedevere-app
Copy link

bedevere-app bot commented Oct 16, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@ember91
Copy link
Contributor Author

ember91 commented Oct 16, 2024

I Have included changes to MISC/HISTORY as well as the Misc/News.d/*.rst files here just to show them. If you think they shouldn't be included because they modify history, just tell me.

I will follow up with more PRs for the remaining file types.

@ember91
Copy link
Contributor Author

ember91 commented Oct 16, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Since this commit/PR only includes documentation only changes, a NEWS addition should not be necessary.

ЉЊЈЁЂ

This doesn't cause a problem in the tect surrounding the examples, but
This doesn't cause a problem in the text surrounding the examples, but
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be test as well but I think it's text.

@rruuaanng
Copy link
Contributor

Recommend add a Docs title label, like this

Docs: Fix typos in documentation files

@ember91 ember91 changed the title Fix typos in documentation files Docs: Fix typos in documentation files Oct 16, 2024
Copy link
Contributor

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

LGTM with one nitpick

Misc/HISTORY Outdated
being subclassed through multiple inheritance.

- Issue #24848: Fixed a number of bugs in UTF-7 decoding of misformed data.
- Issue #24848: Fixed a number of bugs in UTF-7 decoding of malformed data.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be reverted. It match commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@sobolevn
Copy link
Member

@ember91, welcome to CPython 👋

I agree with @willingc, please, don't create large PRs since they also spawn notifications for a lot of codeowners for the changed files.

The only thing that I would like to highlight is that changing Misc/ directory is fine, because it is just for docs.

@ember91
Copy link
Contributor Author

ember91 commented Oct 16, 2024

@ember91, welcome to CPython 👋

I agree with @willingc, please, don't create large PRs since they also spawn notifications for a lot of codeowners for the changed files.

The only thing that I would like to highlight is that changing Misc/ directory is fine, because it is just for docs.

Sorry about that. If you want to I can split this up into one PR per codeowner. Or if you think the damage is done already we can keep it as is.

.. nonce: CLz6qy
.. section: Build
Rename --with-optimiations to --enable-optimizations.
Copy link
Member

Choose a reason for hiding this comment

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

FWIW the spelling fix is appropriate. This is the original commit in 3.5: https://hg.python.org/cpython/rev/c0ea81315fb6

ЉЊЈЁЂ

This doesn't cause a problem in the tect surrounding the examples, but
This doesn't cause a problem in the test surrounding the examples, but
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This doesn't cause a problem in the test surrounding the examples, but
This doesn't cause a problem in the text surrounding the examples, but

I agree with your previous fix. C is closer to X than S on my keyboard. The examples are the doc-tests, embedded in the text.

@ember91
Copy link
Contributor Author

ember91 commented Oct 16, 2024

Closing this PR. I will follow the suggestions by @willingc in #125569 (review), but also its follow up #125569 (comment).

I will apply all suggestions in this PR this far in new PRs, obviously.

@ember91 ember91 closed this Oct 16, 2024
@willingc
Copy link
Contributor

Thanks @ember91 for being receptive to our feedback. Looking forward to your continued contributions.

@ember91 ember91 mentioned this pull request Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants