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

Modify ci.yml 'on' to run on pull request; Add Linux #100

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

baconpaul
Copy link
Collaborator

@baconpaul baconpaul commented Mar 12, 2024

Closes #93

Lets see if this works!

@baconpaul
Copy link
Collaborator Author

There you go! This is now running the CI on a PR

@baconpaul
Copy link
Collaborator Author

OK re-pushed to add ubuntu also. Lets see!

@sudara
Copy link
Owner

sudara commented Mar 12, 2024

This is now running the CI on a PR

To a be a jerk, it probably was going to work anyway, as you were no longer a first time contributor!

But you are right, pull_request was removed here, maybe due to some annoyance with it firing twice with push sometimes?

OK re-pushed to add ubuntu also.

letsgooooo, going for gold!!!

@baconpaul
Copy link
Collaborator Author

yeah lemme try a bit!

@sudara
Copy link
Owner

sudara commented Mar 12, 2024

i think that warning/error was mentioned here: #93 (comment)

I thiiiink related to the precompiled binary asset stuff — it might need a bit of cleanup, but lol, surprise it's managed by copy_cmake_assets.rb

@baconpaul
Copy link
Collaborator Author

OK looks like that will work. let me squash it and rewrite the comment.

@baconpaul
Copy link
Collaborator Author

OK this is ready!

@baconpaul
Copy link
Collaborator Author

Oooh actually hold on a second.

@baconpaul
Copy link
Collaborator Author

So the precompiled thing. You are treating them as part of your main module whereas juce cmake usually makes them a separate library it compiles itself (with different flags). So it ejects code which doesn't work with error and standard juce warnings.

That's OK though. You can wrap the inclusion of the CPP in a suppress-that-error GCC block I think and have a change to do that

Add edgeGap -> _edgeGap in the ctor argument and in theory you can run linux CI with warnings are errors on.

Lets see!

ci.yml runs on PR and ci turns on ubuntu

Adjust the code to compile with Werror on linux
  1. Remove a shadow edgeGap
  2 .For the binary assets created externally by projucer,
     supress the redundant-decls warning that code has and
     ignores when compiled externally

Closes sudara#93
@baconpaul
Copy link
Collaborator Author

OK now this is really really good to go! :)

@baconpaul baconpaul changed the title Modify ci.yml 'on' to run on pull request Modify ci.yml 'on' to run on pull request; Add Linux Mar 12, 2024
Copy link
Owner

@sudara sudara left a comment

Choose a reason for hiding this comment

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

Amazing.

@sudara sudara merged commit a06c186 into sudara:main Mar 12, 2024
12 checks passed
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.

Add Linux to CI matrix
2 participants