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

add usage of run-cmake action for CI #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lukka
Copy link

@lukka lukka commented Jan 19, 2020

Hello,
here there is a modified version of the CI build, in detail:

  • the workflow execute build and test for the major platforms: linux, macos, windows;
  • the workflow only builds and test, it does not package nor install on the build server; on CI it should not be needed to package and install the package (especially if the package is not automatically tested anyway). Also this make the CI shorter in time; it would make sense to package the product in build on push on master branch though.
  • the workflow uses a specific run-cmake action to configure and build in one step; the run-cmake action provision automatically 'ninja' if not present and it integrates with vcpkg (if used) using the run-vcpkg action;

There might be several reasons that would block this PR or that request heavy changes: I am fine with this of course!

@lukka
Copy link
Author

lukka commented Feb 4, 2020

@kracejic any input on this PR?

@kracejic
Copy link
Owner

Hi, sorry for the late answer, but I am quite busy at work and I wanted to really think about this...

While I very much like building the Windows and Mac version with one action approach, I do not think that adding dependency to external actions would be good step for small simple cmake application skeleton. I was already quite hesitant to add docker image of my own making that lives outside of the repository to the gitlab-ci.yml file...

But thanks a lot for the contribution, it made me look more on the github actions! Have a good one!

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