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

[macOS] Update Vulkan SDK install script. #626

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

trashguy
Copy link

@trashguy trashguy commented Oct 9, 2024

Latest version of SDK switched from DMG to ZIP.

@Bioblaze Bioblaze merged commit b52811b into Redot-Engine:master Oct 9, 2024
19 checks passed
@Spartan322
Copy link
Member

Isn't this gonna create a merge conflict with godotengine/godot#97981 if we sync with the Godot master branch? Could we please not merge Godot PRs that get merged into Godot.

@AwesomeParley
Copy link

Isn't this gonna create a merge conflict with godotengine/godot#97981 if we sync with the Godot master branch? Could we please not merge Godot PRs that get merged into Godot.

To be fair, we are going to go a different direction than Godot, so eventually we are going to not be able to sync. This is a small thing that can easily just get situated while syncing, and thanks to you (I'm being genuine), it makes it easier to see what the issue is, and make syncing a smoother process if/when we do sync next.

@Spartan322
Copy link
Member

Spartan322 commented Oct 9, 2024

The problem is that if something is already merged into Godot, making a PR and merging it is pointless and needlessly disruptive to keeping up with upstream for no reason, and we're not gonna be going out of sync in any way for a long while. It only even makes sense to stay out of sync with Godot when we introduce something new not included in Godot or that was in a rejected or excessively old PR for Godot, we should not focus on merging the highly active and new PRs. (least not without very good reason to do so, like if its extremely necessary for some reason and Godot hasn't yet merged it yet)

That PR was merged before this PR was even created, there was no reason for us to need to merge this over just syncing to upstream.

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.

7 participants