Skip to content

dev.py: alias build with install and add argument to only test specific modules with all #3529

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

damusss
Copy link
Member

@damusss damusss commented Jul 12, 2025

Reasoning:

  • usually building stops at compiling. new contributors might not choose it thinking it won't install pygame - but it will. I have one "documented" case of this happening.
  • when adding some features you usually don't need to test all modules. it would not be a problem if display tests didn't run fullscreen operations, that completely fuck up every other window state because it sucks for some reason. that annoying behavior might discourage the usage of all, and it should not be the case.

@damusss damusss requested a review from a team as a code owner July 12, 2025 17:45
@Starbuck5
Copy link
Member

Starbuck5 commented Jul 14, 2025

I'm sort of -1.

The compilation guide says to use dev.py build - https://github.com/pygame-community/pygame-ce/wiki/Advanced-compilation-guide-%28for-developers%29#building-pygame . I'm curious about the situation where someone sees the command but doesn't read the description and decides not to use it.

Also it's fine not to use it, using pip install . is just slow, not wrong.

Regarding all, I've never used that command, but isn't the point to have a full checklist of things the PR needs ready to go? Not running certain tests seems antithetical.


But really this is Ankith's territory to review.

@damusss
Copy link
Member Author

damusss commented Jul 14, 2025

I'm curious about the situation where someone sees the command but doesn't read the description and decides not to use it.

I mean you are right about that documentation being pretty clear. I guess it should be linked to new developers instead of just telling them to use dev.py (I suppose that might have happened with the person I'm talking about). I still find the alias helpful tho

Regarding all, I've never used that command, but isn't the point to have a full checklist of things the PR needs ready to go? Not running certain tests seems antithetical.

What you say makes sense, but at the same time I don't really want to break the maximized status of ALL my open windows just because I want to make a commit. Maybe it's just me being "greedy", but it's annoying. Either a --skip-fullscreen option can exists (but sounds like too much work for a simple thing like this) or I think it's not bad to constrain the tests only. When you work on some things, there are only so much modules that could possibly fail. As the developer you can probably figure out if that's the case or not, and if you misscalculated the github checks are gonna stop you anyways. But as you said, this is more ankith territory, so let's wait.

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.

I'm basically +0 on this proposal. I don't see the strong need for these changes but at the same time adding this can do no harm. As long as the existing stuff keeps working I don't mind people extending dev.py with whatever extras they want.

Sign up for free to 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