Skip to content

gh-123165: make dis functions render positions on demand - #123168

Merged
iritkatriel merged 30 commits into
python:mainfrom
picnixz:dis-show-positions-123165
Aug 21, 2024
Merged

gh-123165: make dis functions render positions on demand#123168
iritkatriel merged 30 commits into
python:mainfrom
picnixz:dis-show-positions-123165

Conversation

@picnixz

@picnixzpicnixz commented Aug 20, 2024•
edited
Loading

Copy link
Copy Markdown
Member

@bedevere-appbedevere-appBot mentioned this pull request Aug 20, 2024
Closed
@picnixz
picnixzforce-pushed the dis-show-positions-123165 branch from be826a5 to a934b4eCompareAugust 20, 2024 11:36
@picnixz
picnixzforce-pushed the dis-show-positions-123165 branch from a934b4e to a8eab11CompareAugust 20, 2024 11:36
@picnixz
picnixz marked this pull request as ready for review August 20, 2024 12:38
iritkatriel
iritkatriel reviewed Aug 20, 2024
View reviewed changes

@iritkatrieliritkatriel 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.

Looks good. A few comments.

Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/whatsnew/3.14.rst Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/test/test_dis.py Outdated
Comment threadLib/dis.py Outdated
@picnixzpicnixz changed the title gh-123165: add the show_positions formal parameter to dis functionsgh-123165: make dis functions render positions on demandAug 21, 2024
iritkatriel
iritkatriel reviewed Aug 21, 2024
View reviewed changes

@iritkatrieliritkatriel 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.

Cool. A few more comments, nothing major.

Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/library/dis.rst
Comment threadDoc/library/dis.rst Outdated
Comment threadDoc/whatsnew/3.14.rst Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
@picnixz
picnixzforce-pushed the dis-show-positions-123165 branch from 112fe26 to ca3cf87CompareAugust 21, 2024 10:02
@picnixz
picnixzforce-pushed the dis-show-positions-123165 branch from 548b30f to 9117895CompareAugust 21, 2024 10:10
@picnixzpicnixz mentioned this pull request Aug 21, 2024
Merged
iritkatriel
iritkatriel reviewed Aug 21, 2024
View reviewed changes

@iritkatrieliritkatriel 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.

Getting there. Some doc tweaks and a question.

Comment threadDoc/whatsnew/3.14.rst Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
Comment threadLib/dis.py Outdated
@picnixz

picnixz commented Aug 21, 2024

Copy link
Copy Markdown
MemberAuthor

I marked your comments as resolved but I hoped I didn't miss any. By the way, thank you very much for your help @iritkatriel!

@iritkatriel

iritkatriel commented Aug 21, 2024

Copy link
Copy Markdown
Member

LGTM. @markshannon is this what you had in mind?

@markshannon

markshannon commented Aug 21, 2024

Copy link
Copy Markdown
Member

Yes

@iritkatriel
iritkatriel merged commit b1d3bd2 into python:mainAug 21, 2024
@iritkatriel

iritkatriel commented Aug 21, 2024

Copy link
Copy Markdown
Member

Thank you @picnixz !

@picnixz
picnixz deleted the dis-show-positions-123165 branch August 21, 2024 13:50
@picnixz

picnixz commented Aug 21, 2024

Copy link
Copy Markdown
MemberAuthor

Thank you for your review as well, Irit!

blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@picnixz@blhsing
pythongh-123165: makedisfunctions render positions on demand (pyt…
966626a
…hon#123168)
@picnixzpicnixz mentioned this pull request Dec 5, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel approved these changes
@markshannonmarkshannonAwaiting requested review from markshannon

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@picnixz@iritkatriel@markshannon