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

Bugfix/font stubs #2358

Merged
merged 12 commits into from
Aug 14, 2023
Merged

Bugfix/font stubs #2358

merged 12 commits into from
Aug 14, 2023

Conversation

dr0id
Copy link
Contributor

@dr0id dr0id commented Jul 29, 2023

follow up PR of #2175 to fix the stubs of all properties in font and freetype

@dr0id dr0id requested a review from a team as a code owner July 29, 2023 20:27
@yunline yunline added type hints Type hint checking related tasks font pygame.font labels Aug 1, 2023
@Starbuck5
Copy link
Member

What am I looking at?

I see a bazillion formatting changes, and attributes are @property not in the font stubs. Why?

@dr0id
Copy link
Contributor Author

dr0id commented Aug 2, 2023

What am I looking at?

I see a bazillion formatting changes, and attributes are @property not in the font stubs. Why?

The changes are in
gen_stubs.py (to be consistent with formatting)
font.pyi
freetype.pyi

All the other changes are just formatting changes after running:
setup.py lint
setup.py format

Not sure why I have those changes, but ci builds seems not to bothered by those changes.

I'm using:
Python 3.10.4
black 23.7.0
clang 16.0.1.1
clang-format 16.0.4

@ankith26
Copy link
Member

ankith26 commented Aug 2, 2023

RN setup.py lint/format doesn't really format the typestubs, perhaps your formatting is a result of something else? Anyways, for ease of review I'd like the formatting changes in a separate PR (I know... It's ironic coming from me 😄)

@dr0id
Copy link
Contributor Author

dr0id commented Aug 2, 2023

RN setup.py lint/format doesn't really format the typestubs, perhaps your formatting is a result of something else? Anyways, for ease of review I'd like the formatting changes in a separate PR (I know... It's ironic coming from me 😄)

You are right, I run (from my setup.py):

python.exe -m black buildconfig\stubs\pygame

Right now from the setup.py only following python directories would be touched:

python_directories = ["src_py", "test", "examples"]

@dr0id
Copy link
Contributor Author

dr0id commented Aug 2, 2023

RN setup.py lint/format doesn't really format the typestubs, perhaps your formatting is a result of something else? Anyways, for ease of review I'd like the formatting changes in a separate PR (I know... It's ironic coming from me 😄)

removed linting changes @ankith26

update return type of style from None to int

Co-authored-by: Ankith <46915066+ankith26@users.noreply.github.com>
freetype.pyi: use ColorValue for color parameters
@dr0id dr0id requested a review from ankith26 August 4, 2023 19:55
change color getters back to return Color instead of ColorValue
@dr0id dr0id requested a review from ankith26 August 6, 2023 19:54
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR 🎉

@ankith26 ankith26 added this to the 2.3.2 milestone Aug 10, 2023
@dr0id dr0id requested a review from yunline August 12, 2023 16:40
@dr0id dr0id requested a review from Starbuck5 August 12, 2023 16:40
@dr0id dr0id self-assigned this Aug 12, 2023
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 👍

Clear what the change is now, and I approve.

@MyreMylar MyreMylar merged commit f32cb9d into pygame-community:main Aug 14, 2023
31 checks passed
ankith26 pushed a commit that referenced this pull request Sep 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
font pygame.font type hints Type hint checking related tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants