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

Test CMake Config script #1421

Merged

Conversation

remia
Copy link
Collaborator

@remia remia commented Jun 22, 2021

This PR is a follow up of #1397, adding a simple test of the generated script.

  • A dummy single file project is built and run after main library has been tested,
  • Currently only enabled on shared build, it seems static build would be harder to test and require the client project to import the correct dependencies,
  • Build is done in source to avoid some inconsistency in GitHub Action while changing directory within a single job (I had some issues in Windows),
  • Added cxx_std_11 public compiler feature on the core library based on @hodoulp suggestion.

On a related note, I added an explicit warning ignore in the client project, the already known C4275. I wonder if we could do that from #pragma inside the source code (OCIO Exception class) to not impose that on the users.

Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
.github/workflows/ci_workflow.yml Outdated Show resolved Hide resolved
.github/workflows/ci_workflow.yml Outdated Show resolved Hide resolved
.github/workflows/ci_workflow.yml Show resolved Hide resolved
tests/cmake-consumer/consumer.cpp Outdated Show resolved Hide resolved
@hodoulp
Copy link
Member

hodoulp commented Jun 23, 2021

On a related note, I added an explicit warning ignore in the client project, the already known C4275. I wonder if we could do that from #pragma inside the source code (OCIO Exception class) to not impose that on the users.

That's a good point. My answer is yes in OpenColorIO.h

Copy link
Collaborator

@michdolan michdolan left a comment

Choose a reason for hiding this comment

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

Great idea @remia !

src/OpenColorIO/CMakeLists.txt Show resolved Hide resolved
Signed-off-by: Rémi Achard <remiachard@gmail.com>
Signed-off-by: Rémi Achard <remiachard@gmail.com>
@remia remia force-pushed the feature/cmake-consumer-test branch from 5a225fe to 22a613d Compare June 28, 2021 14:21
@michdolan michdolan merged commit ceec84d into AcademySoftwareFoundation:master Jul 26, 2021
@remia remia deleted the feature/cmake-consumer-test branch August 24, 2021 11:08
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.

3 participants