Skip to content

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga added this to the 4.x milestone Oct 9, 2025
@akien-mga akien-mga requested a review from BlueCube3310 October 9, 2025 10:41
@akien-mga akien-mga requested a review from a team as a code owner October 9, 2025 10:41
@akien-mga akien-mga force-pushed the basisu-git-2025-10-09 branch from 9e42d79 to 845f9f7 Compare October 9, 2025 10:42

#ifndef __EMSCRIPTEN__
#if defined(__GNUC__) && !defined(__clang__)
#ifdef __GNUC__
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI @Repiteo we didn't need that change. This is a .cpp file so it's compiled in env_thirdparty with disable_warnings() so it shouldn't raise warnings for us.

The case where we get warnings from third-party code is when they're emitted by headers which we include in first-party Godot code compiled with warnings enabled. That's what -isystem etc. aimed to work around, by marking third-party headers as "system" so they don't generate internal warnings.

Anyway I proposed a fix for that one upstream too:

But I'm not including it in this PR as we don't need it, and we want to minimize the amount of patches we have to maintain downstream.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shoot, so it is! Must've forgot to limit my regex search to headers

Copy link
Contributor

@Repiteo Repiteo 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!

@Repiteo Repiteo modified the milestones: 4.x, 4.6 Oct 9, 2025
@Repiteo Repiteo merged commit db34073 into godotengine:master Oct 9, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 9, 2025

Thanks!

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.

2 participants