Skip to content

Separate setup-godot-cpp action from build action, to allow for more customizability#68

Closed
Ivorforce wants to merge 1 commit intogodotengine:mainfrom
Ivorforce:action-setup-separate
Closed

Separate setup-godot-cpp action from build action, to allow for more customizability#68
Ivorforce wants to merge 1 commit intogodotengine:mainfrom
Ivorforce:action-setup-separate

Conversation

@Ivorforce
Copy link
Member

@Ivorforce Ivorforce commented Nov 10, 2024

This is coming from the same idea as #66, but proposes a different approach to achieve customizability.

I think I like this one more, since a lot of parameters were only there to be passed to scons unchanged. There's a reason github actions often follow the setup-xxx formula - it's just a good fit, I think.

I don't think the build action is needed still in this PR, but I kept it around for backwards compatibility.

@Ivorforce Ivorforce force-pushed the action-setup-separate branch from 5cbf5c7 to 2109cd9 Compare November 10, 2024 23:25
@dsnopek
Copy link
Contributor

dsnopek commented Nov 12, 2024

Moving this to its own action is fine, and I think the name is OK.

However, while this is still part of the godot-cpp-template repo, I don't think we should be looking at this in terms of "customizability" and "backwards compatibility". This is a template that folks are meant to copy once, and then do whatever they want with. The way to customize it is to change the action itself, and there is no backwards compatibility because no one should be pulling in future changes from the template.

If we want to have a reusable action, then I think this should be moved to its own repo and maintained outside of the template project.

@Ivorforce
Copy link
Member Author

As per GDExtension meeting, we will move the github setup action to godot-cpp. This will be better represented by another PR.

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.

2 participants