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

Object creator fix #71140

Merged

Conversation

physics-enthusiast
Copy link
Contributor

Summary

None

Purpose of change

Fix the failing object creator release builds. Not actually sure what they are for, but since they are there I assume them not failing is preferable.

Describe the solution

Installs Qt5 in the linux devcontainer. I think they were mistakenly moved into the windows one when it was separated. Get the windows job to actually use the correct container.

Testing

Error goes away.

@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Jan 21, 2024
@harakka
Copy link
Member

harakka commented Jan 22, 2024

Can you please add a separate build container dockerfile instead of adding these deps the current default devcontainer one? They were split is so that people don't need to wait for a ton of unnecessary deps (that being qt5) to be installed to work on the game itself, and the wait was long enough that using the devcontainer in GitHub codespaces was timing out.

@physics-enthusiast
Copy link
Contributor Author

Should the default container still be built and cached as part of the release process? Since with my proposed changes the devcontainers for 2 object creator builds (whose purpose I'm still not entirely clear on, for the record) are decoupled from default devcontainer that people use for codespaces. Might still be useful for those in the CleverRaven organization proper and whoever also happens to have been granted access to its container registry. Another possibility is for the cached containers to be made publically accessible so all people have to do is pull the image without building it again, although I haven't actually looked too deep into the discourse around our devcontainers in general so IDK what the current senior dev opinion on this is.

@harakka
Copy link
Member

harakka commented Jan 22, 2024

The dev containers aren't really intended for building in CI for the first place, but to ease local development (although I do in principle agree it's good to share definitions for build and dev toolchains). They're not too widely used but they help the group of people get stuff done who don't have the full toolchain installed locally and work in VS Code or in Codespaces.

Only very small chunk of devs has access to the CleverRaven repo, I don't think there's much value for building the images just for that, so I don't see value in building the ones not used for CI actions.

The object creator is a GUI editor for some very limited subset of our json definitions.

As an aside, it was briefly discussed in devcord recently that the object creator failing to build necessarily shouldn't block the Experimental Release workflow from being considered successful.

@BrettDong
Copy link
Member

You need to rebase the branch to a more recent master to pick up the fix for the broken basic build CI.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 26, 2024
@kevingranade kevingranade merged commit 21c62bd into CleverRaven:master Jan 29, 2024
24 checks passed
@physics-enthusiast physics-enthusiast deleted the object-creator-fix branch January 29, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants