Ozdasc/version bump #2191
Ozdasc/version bump #2191
Conversation
Signed-off-by: cuneyt.ozdas <[email protected]>
…mp-dependencies - Few tweaks
Signed-off-by: cuneyt.ozdas <[email protected]>
…rsionBumpMinorFixes02 - Few more minor tweaks
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR performs a comprehensive version bump across the OCIO codebase, updating supported C++ standards, dependency versions, and Python versions to modernize the project.
- Upgrades minimum C++ standard from 14 to 17 and adds support for C++20/23
- Updates all major dependencies to their latest versions (yaml-cpp 0.8.0, expat 2.7.2, etc.)
- Drops Python 3.7/3.8 support and adds Python 3.14 support
Reviewed Changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| share/cmake/utils/CppVersion.cmake | Updates supported C++ standards and default version handling |
| share/cmake/modules/FindExtPackages.cmake | Bumps minimum and recommended versions for all dependencies |
| share/cmake/modules/install/Installyaml-cpp.cmake | Updates yaml-cpp installation and git tag format |
| tests/cpu/fileformats/FileFormatCTF_tests.cpp | Adjusts test expectations for library version differences |
| src/OpenColorIO/SystemMonitor_macos.cpp | Replaces VLA with std::vector for C++ compliance |
| src/apps/ocioconvert/main.cpp | Adds Metal support for Apple platforms |
| Multiple CMakeLists.txt files | Updates library linking from legacy variables to modern targets |
| Multiple CI/workflow files | Updates Python versions, dependency versions, and build configurations |
| Documentation files | Updates version requirements in installation docs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| std::vector<char> buffer(bufferSize); | ||
|
|
||
| // Return false if the buffer is too small or if the conversion fails. | ||
| if (CFStringGetCString(values[0], buffer, bufferSize, kCFStringEncodingUTF8)) | ||
| if (CFStringGetCString(values[0], buffer.data(), buffer.size(), kCFStringEncodingUTF8)) | ||
| { | ||
| // Build a name using the vendor information. | ||
| displayName = buffer; | ||
| displayName = buffer.data(); |
There was a problem hiding this comment.
Consider using std::string instead of std::vector for better semantics and to avoid the .data() calls, or reserve the exact size if the buffer size is known to be large.
| "${PLATFORM_COMPILE_OPTIONS};/D_SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING" | ||
| ) | ||
| endif() | ||
| # seems no longer needed, there were c++ modernization on 0.7.0. /coz |
There was a problem hiding this comment.
The comment has grammatical issues. Should be: '# seems no longer needed, there was C++ modernization in 0.7.0. /coz'
| # seems no longer needed, there were c++ modernization on 0.7.0. /coz | |
| # seems no longer needed, there was C++ modernization in 0.7.0. /coz |
| set(Python_PRE_CMD set ${_PATH_SET} "\n" set ${_PYTHONPATH_SET} "\n" call) | ||
| set(Python_PRE_CMD set ${_PYTHONPATH_SET} "\n" call) | ||
|
|
||
| message(STATUS "Python pre-command: ${Python_PRE_CMD}") |
There was a problem hiding this comment.
Debug message statements are being added to production code. Consider removing these STATUS messages or wrapping them in a debug flag check.
| string(JOIN ":" _PYTHONPATH_VALUE ${_PATHS}) | ||
| set(Python_PRE_CMD "PYTHONPATH=${_PYTHONPATH_VALUE}") | ||
|
|
||
| message(STATUS "Python pre-command: ${Python_PRE_CMD}") |
There was a problem hiding this comment.
Debug message statements are being added to production code. Consider removing these STATUS messages or wrapping them in a debug flag check.
Signed-off-by: Cuneyt Ozdas <[email protected]>
|
Wrong target repo. Will open later, when it's ready. |
2 participants
No description provided.