Skip to content

Conversation

@picnixz
Copy link
Member

@picnixzpicnixz commented Aug 20, 2024

@picnixzpicnixzforce-pushed the dis-show-positions-123165 branch from be826a5 to a934b4eCompareAugust 20, 2024 11:36
@picnixzpicnixzforce-pushed the dis-show-positions-123165 branch from a934b4e to a8eab11CompareAugust 20, 2024 11:36
@picnixzpicnixz marked this pull request as ready for review August 20, 2024 12:38
Copy link
Member

@iritkatrieliritkatriel 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. A few comments.

@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
Copy link
Member

@iritkatrieliritkatriel left a comment

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.

@picnixzpicnixzforce-pushed the dis-show-positions-123165 branch from 112fe26 to ca3cf87CompareAugust 21, 2024 10:02
Copy link
Member

@iritkatrieliritkatriel left a comment

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.

@picnixz
Copy link
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
Copy link
Member

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

@markshannon
Copy link
Member

Yes

@iritkatrieliritkatriel merged commit b1d3bd2 into python:mainAug 21, 2024
@iritkatriel
Copy link
Member

Thank you @picnixz !

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

Thank you for your review as well, Irit!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@picnixz@iritkatriel@markshannon