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

GH-101777: queue.rst: use 2 spaces after a period to be consistent. #102143

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

OTheDev
Copy link
Contributor

@OTheDev OTheDev commented Feb 22, 2023

In #102026, I put one space after a period instead of two. The latter is used throughout the file. This PR reintroduces the space that I removed.

cc @ericvsmith

EDIT: After looking a bit closer, there are other instances where a single space is used after a sentence-ending period rather than two. If desired, I can change those too.

@ericvsmith
Copy link
Member

If you saw another instance, then yes, I think you should correct that one, too.

@OTheDev OTheDev changed the title PriorityQueue docs. Use 2 spaces after a period to be consistent. queue.rst: use 2 spaces after a period to be consistent. Feb 22, 2023
@OTheDev
Copy link
Contributor Author

OTheDev commented Feb 22, 2023

The most recent commit ensures this rule is followed consistently across the file.

There are other issues I found. For example, the maximum line length of 80 stipulated in the devguide is not adhered to throughout the file. But I haven't seen this rule consistently enforced. My aim was to keep the diff to a minimum.

Copy link
Member

@ericvsmith ericvsmith left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Feb 22, 2023

Can we please not start encouraging PRs like this? IMO this is churn and nothing else. Can we please just close this PR, @ericvsmith? (Sorry if I'm coming out harsh.)

@rhettinger rhettinger changed the title queue.rst: use 2 spaces after a period to be consistent. GH-107177: queue.rst: use 2 spaces after a period to be consistent. Feb 22, 2023
@hugovk
Copy link
Member

hugovk commented Feb 22, 2023

Generally speaking, I wouldn't bother going through fixing more: we usually aim to avoid churn, this doesn't make any difference to end users, and the devguide says to use one or two.

A sentence-ending period may be followed by one or two spaces; while reST ignores the second space, it is customarily put in by some users, for example to aid Emacs’ auto-fill mode.

@rhettinger rhettinger changed the title GH-107177: queue.rst: use 2 spaces after a period to be consistent. GH-101777: queue.rst: use 2 spaces after a period to be consistent. Feb 22, 2023
@erlend-aasland
Copy link
Contributor

(For the record, I'm not criticising you for anything, @OTheDev)

@erlend-aasland
Copy link
Contributor

@OTheDev:

My aim was to keep the diff to a minimum.

BTW, that is a very good rule to follow for future PRs, both for code and documentation.

@rhettinger
Copy link
Contributor

@erlend-aasland In general, I agree that this is mostly a waste of time, but this was specifically requested by a senior core dev in another issue.

@rhettinger rhettinger merged commit 96bf243 into python:main Feb 22, 2023
@erlend-aasland
Copy link
Contributor

erlend-aasland commented Feb 22, 2023

@rhettinger, please do not merge PRs marked with DO-NOT-MERGE. Merging such PRs is an encouragement for churn in itself.

[…] this was specifically requested by a senior core dev in another issue.

That request was specifically countered by three other core devs. Not exactly what I'd call consensus.

@ericvsmith
Copy link
Member

I requested the original change because the PR I was responding to removed a second space, which made the style inconsistent within that file. I don't think it's a problem to restore the recently made inconsistent change, and I also don't think it's a big deal to change the other few places that file was inconsistent.

@gvanrossum
Copy link
Member

Let’s just not encourage such PRs in the future. They really don’t add any value to the code base, they introduce noise in the commit history, and waste resources.

Also note the prominent heading in PEP 8, https://peps.python.org/pep-0008/#a-foolish-consistency-is-the-hobgoblin-of-little-minds.

@erlend-aasland
Copy link
Contributor

Also, this now has to be backported, in order to minimise conflicts with future backports, thus creating even more churn.

@erlend-aasland erlend-aasland added the needs backport to 3.10 only security fixes label Feb 22, 2023
@erlend-aasland erlend-aasland added the needs backport to 3.11 only security fixes label Feb 22, 2023
@miss-islington
Copy link
Contributor

Thanks @OTheDev for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @OTheDev for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 22, 2023
…stent. (pythonGH-102143)

(cherry picked from commit 96bf243)

Co-authored-by: Owain Davies <[email protected]>
@bedevere-bot
Copy link

GH-102154 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Feb 22, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 22, 2023
…stent. (pythonGH-102143)

(cherry picked from commit 96bf243)

Co-authored-by: Owain Davies <[email protected]>
@bedevere-bot
Copy link

GH-102155 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Feb 22, 2023
miss-islington added a commit that referenced this pull request Feb 22, 2023
miss-islington added a commit that referenced this pull request Feb 22, 2023
carljm added a commit to carljm/cpython that referenced this pull request Feb 23, 2023
* main: (76 commits)
  Fix syntax error in struct doc example (python#102160)
  pythongh-99108: Import MD5 and SHA1 from HACL* (python#102089)
  pythonGH-101777: `queue.rst`: use 2 spaces after a period to be consistent. (python#102143)
  Few coverage nitpicks for the cmath module (python#102067)
  pythonGH-100982: Restrict `FOR_ITER_RANGE` to a single instruction to allow instrumentation. (pythonGH-101985)
  pythongh-102135: Update turtle docs to rename wikipedia demo to rosette (python#102137)
  pythongh-99942: python.pc on android/cygwin should link to libpython per configure.ac (pythonGH-100356)
  pythongh-95672 fix typo SkitTest to SkipTest (pythongh-102119)
  pythongh-101936: Update the default value of fp from io.StringIO to io.BytesIO (pythongh-102100)
  pythongh-102008: simplify test_except_star by using sys.exception() instead of sys.exc_info() (python#102009)
  pythongh-101903: Remove obsolete undefs for previously removed macros Py_EnterRecursiveCall and Py_LeaveRecursiveCall (python#101923)
  pythongh-100556: Improve clarity of `or` docs (python#100589)
  pythongh-101777: Make `PriorityQueue` docs slightly clearer (python#102026)
  pythongh-101965: Fix usage of Py_EnterRecursiveCall return value in _bisectmodule.c (pythonGH-101966)
  pythongh-101578: Amend exception docs (python#102057)
  pythongh-101961 fileinput.hookcompressed should not set the encoding value for the binary mode (pythongh-102068)
  pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)
  pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)
  pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)
  pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)
  ...
JelleZijlstra pushed a commit to JelleZijlstra/cpython that referenced this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants