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.

Sign up for GitHub

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

Jump to bottom

Improved blits() docs #2263

Merged
merged 3 commits into from
Aug 14, 2023
Merged

Improved blits() docs #2263

merged 3 commits into from
Aug 14, 2023

Conversation

itzpr3d4t0r
Copy link
Member

@itzpr3d4t0r itzpr3d4t0r commented Jun 19, 2023 •
edited
Loading

Follows #2259. This one aims to improve the Surface.blits() method docs.

Added:

  • A better and more natural explaination of what the function parameters are and what they do.
  • A thorough explaination of what each item in the blit_sequence can be composed of and its resulting behaviour.
  • A series of notes regarding suggestions about its use/when not to use blits over blit or fblits.

Removed:

  • The compact parameters version, as they weren't used extensively in the surface module and forced having shorter descriptions.

Before:
image

After:
image

@itzpr3d4t0r itzpr3d4t0r requested a review from a team as a code owner June 19, 2023 20:06
oddbookworm
oddbookworm previously requested changes Jul 1, 2023
View reviewed changes
Copy link
Member

@oddbookworm oddbookworm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I don't see any problems with this other than the comment I left

docs/reST/ref/surface.rst Show resolved Hide resolved
@itzpr3d4t0r itzpr3d4t0r mentioned this pull request Jul 1, 2023
Closed
Matiiss
Matiiss reviewed Jul 8, 2023
View reviewed changes
Copy link
Member

@Matiiss Matiiss left a comment

Choose a reason for hiding this comment

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

Looks pretty cool.
One more thing that should be changed is on line 136 it should probably be doreturn=True in the signature instead of 1, since, well, True is more Pythonic.

docs/reST/ref/surface.rst Show resolved Hide resolved
docs/reST/ref/surface.rst Outdated Show resolved Hide resolved
Matiiss
Matiiss approved these changes Jul 10, 2023
View reviewed changes
Copy link
Member

@Matiiss Matiiss left a comment

Choose a reason for hiding this comment

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

LGTM

MyreMylar
MyreMylar approved these changes Aug 14, 2023
View reviewed changes
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

MyreMylar
MyreMylar approved these changes Aug 14, 2023
View reviewed changes
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Documentation seems clearer.

@MyreMylar MyreMylar added this to the 2.3.2 milestone Aug 14, 2023
@MyreMylar MyreMylar merged commit df8be4c into pygame-community:main Aug 14, 2023
@ankith26 ankith26 modified the milestones: 2.3.2, 2.4.0 Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Matiiss Matiiss Matiiss approved these changes

@MyreMylar MyreMylar MyreMylar approved these changes

@oddbookworm oddbookworm Awaiting requested review from oddbookworm

Assignees
No one assigned
Labels
docs
Projects
None yet
Milestone
2.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@itzpr3d4t0r @MyreMylar @oddbookworm @Matiiss @ankith26