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 CI build option #551

Merged
merged 2 commits into from
Sep 10, 2018

Conversation

scooperly
Copy link
Contributor

Seeing as how we test very few build combinations through CI, I feel its best not to completely halt compilation for someone else that is trying to build OCIO for themselves. Even one minor version up in GCC produces warnings.

So submitting this to limit -Werror purely to contributors.

@scoopxyz
Copy link
Contributor

Any opinions on this? I'm not 100% on the option name "OCIO_CI" maybe "WERROR" makes more sense.

But in practice, I still don't think its a good idea to halt compilation for other folks. Unless someone knows some easy CMake-foo to override this for your average CMake user.

@hodoulp
Copy link
Member

hodoulp commented Jul 12, 2018

There are two parts to your concern:

  1. 'Warnings as Errors' is a powerful trick to impose some thinking on warnings. Targeting several platforms and compilers it could greatly help ‘discovering’ bugs. I think that OCIO should enforce the ‘Warnings as Errors’ to all parts of the code (including Python bindings, apps…). But I would not disagree to have a flag to disable that behavior on demand. So, I would prefer something like ‘OCIO_WARNING_AS_ERROR’ with true by default.
  2. The CI build do not cover all possible platforms/compilers/versions which is normal. But it could be certainly improved by adding more compiler/platform versions.

@scoopxyz
Copy link
Contributor

I'm sold on your suggestion, I'll revise the PR with that

@dbr
Copy link
Contributor

dbr commented Aug 31, 2018

Would it make sense to disable -Werror based on CMAKE_BUILD_TYPE ?

@michdolan michdolan merged commit 43a543b into AcademySoftwareFoundation:master Sep 10, 2018
scoopxyz added a commit that referenced this pull request Sep 16, 2018
fnordware pushed a commit to fnordware/OpenColorIO that referenced this pull request Oct 8, 2019
* Add optional warning as error

* reformating - whitespace removal
fnordware pushed a commit to fnordware/OpenColorIO that referenced this pull request Oct 8, 2019
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.

None yet

5 participants