Skip to content

Conversation

@jlskuz
Copy link
Contributor

@jlskuz jlskuz commented Mar 27, 2025

Summarize your change.

For (Linux) distro packaging it is common to use system packages instead of submodules and co.

With this option one can ask to use the system rapidjson with CMake flag instead of maintaining custom patches such as https://build.opensuse.org/projects/KDE:Applications/packages/opentimelineio/files/0001-Use-system-rapidjson.patch?expand=1

Note: if OTIO_FIND_RAPIDJSON is ON but rapidjson is not found cmake fails. This behavior is different to OTIO_FIND_IMATH where it will just continue with src/deps as a fallback. However I think if the user explicitly request the system package it makes more sense to fail if not found. In my opinion this should be changed for Imath as well, but that is offtopic for this PR

@github-actions github-actions bot added the submodules Pull requests that update Submodules code label Mar 27, 2025
Signed-off-by: Julius Künzel <julius.kuenzel@kde.org>
@codecov-commenter
Copy link

codecov-commenter commented Mar 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.72%. Comparing base (c0e97b0) to head (b8590aa).
⚠️ Report is 92 commits behind head on main.

❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1859      +/-   ##
==========================================
+ Coverage   84.11%   84.72%   +0.60%     
==========================================
  Files         198      177      -21     
  Lines       22241    12807    -9434     
  Branches     4687     1191    -3496     
==========================================
- Hits        18709    10851    -7858     
+ Misses       2610     1773     -837     
+ Partials      922      183     -739     
Flag Coverage Δ
py-unittests 84.72% <ø> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 131 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f2c378...b8590aa. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jlskuz jlskuz force-pushed the work/find-rapidjson branch from 94ba8d4 to 30812bb Compare March 27, 2025 22:33
@jlskuz
Copy link
Contributor Author

jlskuz commented Mar 27, 2025

The readthedocs.org failure seems unrelated?

@darbyjohnston
Copy link
Contributor

Unrelated; sometimes readthedocs has these failures, possibly a network error. I'm not sure how to re-run the job, it's separate from the Github actions. Maybe wait for a bit and try pushing another change to try again.

@ssteinbach
Copy link
Collaborator

I restarted the RTD build - sometimes that does flake out, sorry

@darbyjohnston
Copy link
Contributor

Thanks for the changes; I agree about the fallback, the Imath behavior could be changed in a separate PR.

@darbyjohnston darbyjohnston merged commit d6fff83 into AcademySoftwareFoundation:main Mar 30, 2025
62 checks passed
@ssteinbach ssteinbach added this to the Public Beta 18 milestone Mar 31, 2025
@ssteinbach ssteinbach changed the title Allow to use rapidjson from the system Allow build system to use rapidjson from the system Mar 31, 2025
@ssteinbach ssteinbach added the build issues building OTIO label Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build issues building OTIO submodules Pull requests that update Submodules code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants