-
Notifications
You must be signed in to change notification settings - Fork 7.4k
Add fast-obj library #49975
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
base: master
Are you sure you want to change the base?
Add fast-obj library #49975
Conversation
@microsoft-github-policy-service agree |
| HEAD_REF master | ||
| ) | ||
|
|
||
| file(COPY "${CMAKE_CURRENT_LIST_DIR}/CMakeLists.txt" DESTINATION "${SOURCE_PATH}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one they already have seems reasonable, why did we need to replace the build system?
This should probably be turned into a PR editing https://github.com/thisistherk/fast_obj/blob/master/CMakeLists.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to get their CMakeLists.txt working with vcpkg (I have to admit I'm not the most knowledgeable when it comes to CMake), and I would like to avoid doing a PR on their repo because they haven't had any activity in 8 months on GH, and they have 3 PRs open for months in the fast_obj repo.
| @@ -0,0 +1,4 @@ | |||
| fast-obj provides CMake targets: | |||
|
|
|||
| find_package(fast-obj CONFIG REQUIRED) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears identical to the generated ones so we shouldn't add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know the text auto generates something. I suggest editing the instructions in Step 4 here to make it more clear for people in the future. I will remove the text
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.jsonmatches what upstream says.vcpkg.jsonmatches what upstream says../vcpkg x-add-version --alland committing the result.