Skip to content

Display list names in Resolution and Discussions-To headers - #2361

Merged
JelleZijlstra merged 4 commits into
python:mainfrom
AA-Turner:discussions-to-display
Feb 27, 2022
Merged

Display list names in Resolution and Discussions-To headers#2361
JelleZijlstra merged 4 commits into
python:mainfrom
AA-Turner:discussions-to-display

Conversation

@AA-Turner

@AA-TurnerAA-Turner commented Feb 25, 2022

Copy link
Copy Markdown
Member

As a follow-up to #2351, this "prettifies" thread links by extracting the list name for display. I also included Resolution as it seemed odd to normalise one but not the other.

As part of this I also normalised all the mailing list values in Discussions-To (first commit) -- this is not strictly essential but is on the "would be nice" list.

A

@AA-TurnerAA-Turner mentioned this pull request Feb 25, 2022
Merged
brettcannon
brettcannon approved these changes Feb 25, 2022
View reviewed changes

@brettcannonbrettcannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't review the code changes, but the PEP changes LGTM.

CAM-Gerlach
CAM-Gerlach reviewed Feb 25, 2022
View reviewed changes

@CAM-GerlachCAM-Gerlach left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your diligent work here; this overall looks excellent and is a significant improvement (over both of our original proposals, and the previous status quo). I checked it on a variety of PEPs with different outputs, and it all looks good, with the exception of two formatting concerns, both of which actually simplify the current code.

Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py Outdated
Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py Outdated
Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py Outdated
JelleZijlstra
JelleZijlstra requested changes Feb 26, 2022
View reviewed changes
Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py Outdated
Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py Outdated
Comment threadpep_sphinx_extensions/pep_processor/transforms/pep_headers.py
CAM-Gerlach
CAM-Gerlach approved these changes Feb 27, 2022
View reviewed changes

@CAM-GerlachCAM-Gerlach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks @AA-Turner for this great improvement.

@JelleZijlstra
JelleZijlstra merged commit 42f88f1 into python:mainFeb 27, 2022
This was referenced Mar 9, 2022
Merged
Merged
@CAM-GerlachCAM-Gerlach mentioned this pull request Mar 25, 2022
Merged
@erlend-aaslanderlend-aasland mentioned this pull request May 13, 2022
Closed
@erlend-aaslanderlend-aasland mentioned this pull request Jun 27, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannonbrettcannonbrettcannon approved these changes
@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes
@CAM-GerlachCAM-GerlachCAM-Gerlach approved these changes
@hugovkhugovkAwaiting requested review from hugovk

Assignees

No one assigned

Labels

CLA signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AA-Turner@brettcannon@JelleZijlstra@CAM-Gerlach@the-knights-who-say-ni