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: scons_version to environment variable #91080

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Apr 23, 2024

Removes uncertainty of _get_major_minor_revision as a static method by making a call via env like before. Instead of it being locked to a local call, it's now treated as a property of env to allow it to be called across multiple scripts if needed.

@Repiteo Repiteo requested a review from a team as a code owner April 23, 2024 21:31
@Sauermann Sauermann added this to the 4.3 milestone Apr 23, 2024
Copy link
Contributor

@Sauermann Sauermann left a comment

Choose a reason for hiding this comment

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

Have tested the PR rebased on c7f56d3
System: Debian GNU/Linux 12 (bookworm) 12 - X11 - GLES3 (Compatibility) - NVIDIA - AMD

Can confirm, that with these changes I can compile Godot again.
Can't say anything about the implementation.

@fire
Copy link
Member

fire commented Apr 24, 2024

We tend to spell out words so I expected to see "scons_ver" -> "scons_version". Comments?

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.

Looks good. Could indeed rename to scons_version as suggested now that's it's not just a local variable.

@akien-mga akien-mga changed the title SCons: scons_ver to environment variable SCons: scons_version to environment variable Apr 24, 2024
@akien-mga akien-mga merged commit fdd159d into godotengine:master Apr 24, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Repiteo Repiteo deleted the scons/scons_ver-variable branch April 24, 2024 13:37
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