Improvement/update deps and more #248
Conversation
WalkthroughThe changes involve updating the commit references for two subprojects: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Assimp
participant CppCore
participant CMake
participant GMock
User->>Assimp: Update commit reference
Assimp-->>User: Commit updated to 2591d0e4902c006dd8366f1d72bf44be1c40cac6
User->>CppCore: Update commit reference
CppCore-->>User: Commit updated to 12b60d1fb17774ae870b30d453fa3d9d36f74726
User->>CMake: Modify CMakeLists.txt
CMake-->>User: CMake configuration updated
User->>GMock: Delete Google Mock files
GMock-->>User: Files deleted
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
CMakeLists.txt (3)
Line range hint
1-9: Improved CMake configuration and versioning.The changes to the CMake version requirement and project versioning are good improvements:
- Using a version range for CMake ensures compatibility with a wider range of CMake versions.
- Breaking down the project version into major, minor, and patch components allows for more granular version control.
Consider using the
project()command to set the version, which is the modern CMake way:project(osre VERSION ${OSRE_VERSION} LANGUAGES CXX)This would allow you to use
PROJECT_VERSIONdirectly without setting it manually.
10-10: Improved C++ standard setting.Updating to C++17 and using
set(CMAKE_CXX_STANDARD 17)is a good change. It allows the use of more modern C++ features and is a more direct way of setting the standard.Consider adding these lines for better control over the C++ standard:
set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF)This ensures that the specified C++ standard is required (not optional) and that compiler-specific extensions are disabled, promoting better portability.
Line range hint
46-75: Improved compiler flags and compatibility checks.The updates to compiler flags and settings are beneficial:
- Enhanced visibility control for GNU and Clang compilers.
- Improved MinGW version checks with appropriate warnings and errors.
- Consistent use of
-fPICflag across different compilers.These changes should improve code portability and catch potential issues earlier in the build process.
Consider adding a check for the C++ standard support in older compilers:
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0) message(WARNING "Your compiler may have incomplete C++17 support. Consider upgrading.") endif()This would help users identify potential compatibility issues with older compiler versions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (43)
- CMakeLists.txt (1 hunks)
- contrib/gmock-1.7.0/CHANGES (0 hunks)
- contrib/gmock-1.7.0/CMakeLists.txt (0 hunks)
- contrib/gmock-1.7.0/CONTRIBUTORS (0 hunks)
- contrib/gmock-1.7.0/LICENSE (0 hunks)
- contrib/gmock-1.7.0/Makefile.am (0 hunks)
- contrib/gmock-1.7.0/Makefile.in (0 hunks)
- contrib/gmock-1.7.0/README (0 hunks)
- contrib/gmock-1.7.0/build-aux/config.guess (0 hunks)
- contrib/gmock-1.7.0/build-aux/config.h.in (0 hunks)
- contrib/gmock-1.7.0/build-aux/config.sub (0 hunks)
- contrib/gmock-1.7.0/build-aux/depcomp (0 hunks)
- contrib/gmock-1.7.0/build-aux/install-sh (0 hunks)
- contrib/gmock-1.7.0/build-aux/missing (0 hunks)
- contrib/gmock-1.7.0/configure.ac (0 hunks)
- contrib/gmock-1.7.0/fused-src/gmock_main.cc (0 hunks)
- contrib/gmock-1.7.0/gtest/CHANGES (0 hunks)
- contrib/gmock-1.7.0/gtest/CMakeLists.txt (0 hunks)
- contrib/gmock-1.7.0/gtest/CONTRIBUTORS (0 hunks)
- contrib/gmock-1.7.0/gtest/LICENSE (0 hunks)
- contrib/gmock-1.7.0/gtest/Makefile.am (0 hunks)
- contrib/gmock-1.7.0/gtest/Makefile.in (0 hunks)
- contrib/gmock-1.7.0/gtest/README (0 hunks)
- contrib/gmock-1.7.0/gtest/aclocal.m4 (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/config.guess (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/config.h.in (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/config.sub (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/depcomp (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/install-sh (0 hunks)
- contrib/gmock-1.7.0/gtest/build-aux/missing (0 hunks)
- contrib/gmock-1.7.0/gtest/cmake/internal_utils.cmake (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest.cbproj (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest.groupproj (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest_all.cc (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest_link.cc (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest_main.cbproj (0 hunks)
- contrib/gmock-1.7.0/gtest/codegear/gtest_unittest.cbproj (0 hunks)
- contrib/gmock-1.7.0/gtest/configure.ac (0 hunks)
- contrib/gmock-1.7.0/gtest/fused-src/gtest/gtest_main.cc (0 hunks)
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-death-test.h (0 hunks)
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-message.h (0 hunks)
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-param-test.h (0 hunks)
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-param-test.h.pump (0 hunks)
💤 Files with no reviewable changes (42)
- contrib/gmock-1.7.0/CHANGES
- contrib/gmock-1.7.0/CMakeLists.txt
- contrib/gmock-1.7.0/CONTRIBUTORS
- contrib/gmock-1.7.0/LICENSE
- contrib/gmock-1.7.0/Makefile.am
- contrib/gmock-1.7.0/Makefile.in
- contrib/gmock-1.7.0/README
- contrib/gmock-1.7.0/build-aux/config.guess
- contrib/gmock-1.7.0/build-aux/config.h.in
- contrib/gmock-1.7.0/build-aux/config.sub
- contrib/gmock-1.7.0/build-aux/depcomp
- contrib/gmock-1.7.0/build-aux/install-sh
- contrib/gmock-1.7.0/build-aux/missing
- contrib/gmock-1.7.0/configure.ac
- contrib/gmock-1.7.0/fused-src/gmock_main.cc
- contrib/gmock-1.7.0/gtest/CHANGES
- contrib/gmock-1.7.0/gtest/CMakeLists.txt
- contrib/gmock-1.7.0/gtest/CONTRIBUTORS
- contrib/gmock-1.7.0/gtest/LICENSE
- contrib/gmock-1.7.0/gtest/Makefile.am
- contrib/gmock-1.7.0/gtest/Makefile.in
- contrib/gmock-1.7.0/gtest/README
- contrib/gmock-1.7.0/gtest/aclocal.m4
- contrib/gmock-1.7.0/gtest/build-aux/config.guess
- contrib/gmock-1.7.0/gtest/build-aux/config.h.in
- contrib/gmock-1.7.0/gtest/build-aux/config.sub
- contrib/gmock-1.7.0/gtest/build-aux/depcomp
- contrib/gmock-1.7.0/gtest/build-aux/install-sh
- contrib/gmock-1.7.0/gtest/build-aux/missing
- contrib/gmock-1.7.0/gtest/cmake/internal_utils.cmake
- contrib/gmock-1.7.0/gtest/codegear/gtest.cbproj
- contrib/gmock-1.7.0/gtest/codegear/gtest.groupproj
- contrib/gmock-1.7.0/gtest/codegear/gtest_all.cc
- contrib/gmock-1.7.0/gtest/codegear/gtest_link.cc
- contrib/gmock-1.7.0/gtest/codegear/gtest_main.cbproj
- contrib/gmock-1.7.0/gtest/codegear/gtest_unittest.cbproj
- contrib/gmock-1.7.0/gtest/configure.ac
- contrib/gmock-1.7.0/gtest/fused-src/gtest/gtest_main.cc
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-death-test.h
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-message.h
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-param-test.h
- contrib/gmock-1.7.0/gtest/include/gtest/gtest-param-test.h.pump
🧰 Additional context used
🔇 Additional comments (2)
CMakeLists.txt (2)
Line range hint
77-117: LGTM: Subdirectory inclusions and build options.The structure of subdirectory inclusions and build options is well-maintained:
- Conditional inclusions allow for flexible builds.
- Options for building player, editor, tests, and samples provide good customization.
- Third-party libraries (cppcore, assimp) are properly included with appropriate options.
This setup provides a good balance between flexibility and structure in the build process.
Line range hint
1-117: Overall: Improved build configuration with good practices.This update to the CMakeLists.txt file brings several improvements:
- Better CMake version handling and project versioning.
- Updated C++ standard to C++17.
- Improved compiler flag management and compatibility checks.
- Maintained flexible build options and proper third-party library integration.
These changes align with modern CMake practices and should improve the project's build process and portability. The suggestions provided in the review comments are minor and aimed at further enhancing the configuration.
|
Summary by CodeRabbit
contrib/assimpandcontrib/cppcoreto improve stability and performance.