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

PEP 685: Copyedit and fix various minor issues following further changes #2428

Merged
merged 3 commits into from
Mar 16, 2022

Conversation

CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented Mar 14, 2022

Clarifications:

  • Refine description of PEP 503 substitution to avoid implying that only the substitution character (-) is replaced or that only collapsing is done, and slightly expand examples.
  • Add that a critical distinction between safe_extra() and PEP 503 is normalizing non-PEP 508 characters, avoid implying that only collapsing (as opposed to both collapsing and replacement) is not done for - and ., and briefly mention the further inconsistency with the function's docstring.
  • Tweak discussion of PyPI results to explicitly make clear that potential "clashes" were global, not project-specifc
  • Clarify what specifically was being followed in Setuptools and that a key factor in going with PEP 503 instead was a lack of real-world backward compat issues

Copyedits:

  • Fix some grammar issues
  • Add clarifying verbiage in a handful of places
  • Avoid repetition in a few places
  • Use more appropriate diction in a couple spots

Fixes:

  • Add a few missing commas
  • Add verbatim around a few appropriate elements
  • Use inline links per PEP 12 and clean Discourse URLs
  • Update Post-History per new PEP 12 guidance
  • Remove empty Open Issues section

@CAM-Gerlach CAM-Gerlach force-pushed the pep-685-copyedit-v2 branch from ba7b80b to 09b5431 Compare March 14, 2022 04:50
pep-0685.rst Outdated Show resolved Hide resolved
pep-0685.rst Outdated Show resolved Hide resolved
@CAM-Gerlach CAM-Gerlach requested a review from brettcannon March 14, 2022 18:30
pep-0685.rst Outdated Show resolved Hide resolved
@brettcannon
Copy link
Member

@CAM-Gerlach just to make sure there's no confusion, I'm letting you merge this since it's your PR. 🙂

@CAM-Gerlach CAM-Gerlach force-pushed the pep-685-copyedit-v2 branch from c7e0f90 to d3a0a26 Compare March 16, 2022 18:13
@CAM-Gerlach
Copy link
Member Author

@brettcannon I was letting you merge it because it was your PEP...heh.

In all seriousness, sorry about that; I can be too conservative about merging things sometimes, especially on this repo.

I reverted the other change you requested and will wait for the current issue with GitHub Actions/webhooks/pages/etc. to be resolved before merging, since when I merged a PR on another repo a short while ago, the GHP deploy never kicked off and the site didn't update, and I see that the builds on this PR pushing just now didn't trigger either.

@brettcannon
Copy link
Member

In all seriousness, sorry about that; I can be too conservative about merging things sometimes, especially on this repo.

NP! If you didn't have commit rights I would have merged it, but since you do I prefer not to do the merge as you might have last-minute tweaks to do (see the miscommunication Jelle and I had I think last month).

@CAM-Gerlach
Copy link
Member Author

NP! If you didn't have commit rights I would have merged it, but since you do I prefer not to do the merge as you might have last-minute tweaks to do (see the miscommunication Jelle and I had I think last month).

No, I totally understand—I was there for that, and I've seen that happen on other PRs here and elsewhere as its easy for there to be a miscommunication. If I'm doing knarly stuff like a rebase or critical changes I'll temporarily mark it as draft to prevent an inadvertent merge, but its a pretty heavy-handed tool to use for a lot of common cases, so that makes sense. Anyway, without further ado...

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.

6 participants