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

SCons: Convert remaining run_in_subprocess to env.Run #89365

Merged

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Mar 10, 2024

A handful stragglers making direct calls to run_in_subprocess were still in the repo. Replaced them with calls to env.Run, like everything else. I believe this means that every single log output is now accounted for, so verbose=no will be a sea of uninterrupted blue.

@Repiteo Repiteo requested review from a team as code owners March 10, 2024 19:41
@AThousandShips AThousandShips added this to the 4.x milestone Mar 10, 2024
@AThousandShips AThousandShips requested a review from a team March 10, 2024 19:55
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Seems fine. Should we then remove run_in_subprocess from platform_methods if it's no longer used?

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Mar 10, 2024
@Repiteo
Copy link
Contributor Author

Repiteo commented Mar 10, 2024

Nah, it's still used; the difference is that it's now used by Run in methods exclusively. It's possible that it might not be needed in the future if the build flakiness that caused it in the first place has been addressed, but that's something that'll need to be looked into more thoroughly

Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

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

Looks fine and dandy :D

@akien-mga akien-mga merged commit 810f127 into godotengine:master Mar 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Repiteo Repiteo deleted the scons/run_in_subprocess-to-env.Run branch March 10, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants