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

Added Line methods and length attribute #3179

Merged

Conversation

MrRedstone058
Copy link
Contributor

@MrRedstone058 MrRedstone058 commented Oct 17, 2024

@MrRedstone058 MrRedstone058 requested a review from a team as a code owner October 17, 2024 11:59
@MrRedstone058 MrRedstone058 changed the title Added Line methods and 'length: attribute Added Line methods and 'length' attribute Oct 17, 2024
@itzpr3d4t0r itzpr3d4t0r added New API This pull request may need extra debate as it adds a new class or function to pygame geometry pygame.geometry labels Oct 17, 2024
@itzpr3d4t0r itzpr3d4t0r added this to the 2.5.3 milestone Oct 17, 2024
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

I hope none of what I said is wrong. Thanks for contributing btw :)

buildconfig/stubs/pygame/geometry.pyi Outdated Show resolved Hide resolved
buildconfig/stubs/pygame/geometry.pyi Outdated Show resolved Hide resolved
buildconfig/stubs/pygame/geometry.pyi Outdated Show resolved Hide resolved
buildconfig/stubs/pygame/geometry.pyi Outdated Show resolved Hide resolved
src_c/line.c Outdated Show resolved Hide resolved
src_c/line.c Outdated Show resolved Hide resolved
src_c/line.c Outdated Show resolved Hide resolved
src_c/line.c Outdated Show resolved Hide resolved
@MrRedstone058
Copy link
Contributor Author

noted, will fix throughout the day.

docs/reST/ref/geometry.rst Outdated Show resolved Hide resolved
@ankith26
Copy link
Member

I would like you or the person merging this PR (via squash) to correctly add co-author information to the ported commits.

@MrRedstone058
Copy link
Contributor Author

I would like you or the person merging this PR (via squash) to correctly add co-author information to the ported commits.

what do you mean?

@ankith26
Copy link
Member

Check out how the other geometry PRs are handled. For instance in #3071 you can see the first commit correctly credits the multiple authors. This is done by adding something like

Co-authored-by: Emc2356 <[email protected]>
Co-authored-by: NovialRiptide <[email protected]>
Co-authored-by: ScriptLineStudios <[email protected]>
Co-authored-by: Avaxar <[email protected]>
Co-authored-by: maqa41 <[email protected]>

to the description/body of the git commit

@MrRedstone058
Copy link
Contributor Author

MrRedstone058 commented Oct 18, 2024

i see. But how would i go about collecting their email addresses? as well as edit the commit?

@damusss damusss dismissed their stale review October 18, 2024 13:23

Changes have been addressed as proposed

@ankith26
Copy link
Member

The block of code with the emails has already been posted by me in my previous message.
Editing the commit requires amend/rebase and a force push.

Okay never mind, let the merger handle it for now, this is just something for you to keep in mind for future PRs

@itzpr3d4t0r itzpr3d4t0r mentioned this pull request Oct 19, 2024
89 tasks
@MrRedstone058
Copy link
Contributor Author

should i just wait for the next pr to fix the geometry.line doc typos or should i fix it before merging?

@oddbookworm
Copy link
Member

should i just wait for the next pr to fix the geometry.line doc typos or should i fix it before merging?

Are the typos related to this PR? If so, then fix them here. If they're unrelated and you just want to fix them, make that a separate PR. It's easier to get things through if you don't push too many unrelated things together

@MrRedstone058
Copy link
Contributor Author

MrRedstone058 commented Oct 19, 2024

the next pr i will make is also regarding line, which will also change the docs for that, but yes the typos are in this pr

@oddbookworm
Copy link
Member

oddbookworm commented Oct 19, 2024

Fix what this PR touches, and then fix what the next PR touches if needed in that PR. If there are others you want to fix, fix them in a separate fix PR

src_c/line.c Outdated Show resolved Hide resolved
Co-authored-by: Emc2356 <[email protected]>
Co-authored-by: NovialRiptide <[email protected]>
Co-authored-by: ScriptLineStudios <[email protected]>
Co-authored-by: Avaxar <[email protected]>
Co-authored-by: maqa41 <[email protected]>
@MrRedstone058 MrRedstone058 changed the title Added Line methods and 'length' attribute Added Line methods and length attribute Oct 19, 2024
src_c/geometry_common.c Outdated Show resolved Hide resolved
src_c/geometry_common.c Show resolved Hide resolved
src_c/line.c Outdated Show resolved Hide resolved
Copy link
Member

@itzpr3d4t0r itzpr3d4t0r left a comment

Choose a reason for hiding this comment

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

Ok LGTM! Thanks for contributing!!

@itzpr3d4t0r
Copy link
Member

Btw this PR is still not crediting me, so whoever merges this should squash the commits and add this:
Co-authored-by: itzpr3d4t0r <[email protected]>

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.

OK, this looks good to me 👍

Docs, tests & stubs in place. I'm not super certain why you'd want to flip a line yet... but I'm sure there is a use.

@itzpr3d4t0r itzpr3d4t0r merged commit b77bf55 into pygame-community:main Dec 1, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
geometry pygame.geometry New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants