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

🔨 Automatically generate resources files with cmake #4159

Merged
merged 5 commits into from
Nov 19, 2022

Conversation

AnotherFoxGuy
Copy link
Contributor

@AnotherFoxGuy AnotherFoxGuy commented Nov 17, 2022

This PR replaces resources/generate_resources.py with a cmake script that will automatically generate the required resource files

Todo:

  • Test on Windows with QtCreator
  • Test on Windows with proper building
  • Test on Linux
  • Test on Mac
  • Maybe move the generated files to the build folder? (Needs to be discussed)
  • Update CHANGELOG.md

Fixes #3949

@pajlada
Copy link
Member

pajlada commented Nov 17, 2022

Looks great, this approach looks good to me. Thank you for taking the effort and looking into this!

Poke me whenever you feel this is ready for a proper review or if you need a Linux tester.

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@dnsge
Copy link
Contributor

dnsge commented Nov 17, 2022

Works on macOS 👍

My editor also finds the ResourcesAutogen.hpp header file with no problem, so no broken autocomplete which is great.

@Felanbird
Copy link
Collaborator

image
conan fail is unrelated to this PR, I've just been ignoring it

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Felanbird
Copy link
Collaborator

QtCreator on windows is 🆗 now

@kornes
Copy link
Contributor

kornes commented Nov 19, 2022

tested 6497570 with vcpkg on windows and works fine
great change 👍

@AnotherFoxGuy
Copy link
Contributor Author

@pajlada This PR should be ready for a review now

@pajlada
Copy link
Member

pajlada commented Nov 19, 2022

Code looks good, testing Linux vim experience now

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Linux vim experience works great 👍

@pajlada
Copy link
Member

pajlada commented Nov 19, 2022

Is there any further testing you would want before haaving this merged in @AnotherFoxGuy ?

@AnotherFoxGuy
Copy link
Contributor Author

Is there any further testing you would want before having this merged?

I think that it has been tested on all platforms now, so I think it is ready to merge

@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@pajlada pajlada merged commit 9f5477c into Chatterino:master Nov 19, 2022
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.

Our resource system is confusing
5 participants