Skip to content

Conversation

@yunline
Copy link
Contributor

@yunline yunline commented Feb 19, 2023

Issue: #1850

@yunline yunline requested a review from a team as a code owner February 19, 2023 07:23
@Starbuck5
Copy link
Member

@ankith26 What do you want to do about the ubuntu-18.04 build here?

@ankith26
Copy link
Member

If we are dropping 3.6 we also have to drop the 18.04 build (gh actions have deprecated it anyways)

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 👍

I can't find any more CPython 3.6 references outside of a single comment (which is still relevant).

@MyreMylar
Copy link
Member

MyreMylar commented Feb 19, 2023

Looking at > 3.7 stuff we could get rid of the blocks like this:

#if PY_VERSION_HEX < 0x03070000
#ifdef WITH_THREAD
    PyEval_InitThreads();
#endif /* WITH_THREAD */
#endif

Search for : 0x03070000 and there are two like this in rwobject.c and _pygame.h

@itzpr3d4t0r
Copy link
Member

yeah the _pygame.h one should be related to the FASTCALL macros we introduced a while back for back compat with 3.6.

@Starbuck5 Starbuck5 added the build Compiling stuff label Feb 20, 2023
@Starbuck5 Starbuck5 added this to the 2.1.4 milestone Feb 22, 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.

Changes look good 👍

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM! But seeing as how major a change this is, I'm not going to merge it because I think we might want at least one more approval?

@oddbookworm oddbookworm linked an issue Feb 23, 2023 that may be closed by this pull request
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.

Overall the PR looks good, left two minor reviews for your consideration. You may choose to fix these in this PR, or (you or anyone else can) do a different PR shortly after merging this.

Thanks for the PR! 🎉

Co-authored-by: Ankith <46915066+ankith26@users.noreply.github.com>
@ankith26 ankith26 merged commit 2065e8d into pygame-community:main Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Compiling stuff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop CPython 3.6 support

6 participants