-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[hlslpp] Add new port #40400
[hlslpp] Add new port #40400
Conversation
@microsoft-github-policy-service agree
Sent from Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: microsoft-github-policy-service[bot] ***@***.***>
Sent: Sunday, August 11, 2024 7:29:20 PM
To: microsoft/vcpkg ***@***.***>
Cc: Pantelis Lekakis ***@***.***>; Mention ***@***.***>
Subject: Re: [microsoft/vcpkg] Hlslpp port (hlsl++ from https://github.com/redorav/hlslpp) (PR #40400)
@plekakis<https://github.com/plekakis> the command you issued was incorrect. Please try again.
Examples are:
@microsoft-github-policy-service agree
and
@microsoft-github-policy-service agree company="your company"
—
Reply to this email directly, view it on GitHub<#40400 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABL7J5PJ2ZPSJF3IAAZZT6TZQ6UQBAVCNFSM6AAAAABMK6L4VWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBSHA2DOOBZHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Co-authored-by: jim wang <[email protected]>
Co-authored-by: jim wang <[email protected]>
Co-authored-by: jim wang <[email protected]>
Note: I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags. |
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.
Most comments don't add value to the command that follow. Just remove them.
The CMake config package is not provided by upstream. The package-name should be prefixed with unofficial-
.
https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#add-cmake-exports-in-an-unofficial--namespace
And even for a header-only package, it might make sense to provide an imported target instead of variables.
ports/hlslpp/hlslpp-config.cmake
Outdated
set(hlslpp_VERSION 3.5) | ||
|
||
# Specify include directories | ||
set(hlslpp_INCLUDE_DIRS "${CMAKE_CURRENT_LIST_DIR}/../../include/hlslpp/include") |
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.
Wouldn't it be nice to place includes in ${CMAKE_CURRENT_LIST_DIR}/../../include/hlslpp
?
ports/hlslpp/portfile.cmake
Outdated
# vcpkg_check_linkage(ONLY_STATIC_LIBRARY) | ||
|
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.
# vcpkg_check_linkage(ONLY_STATIC_LIBRARY) | |
# vcpkg_check_linkage(ONLY_STATIC_LIBRARY) | |
Apologies for all the above, this is my first vcpkg contribution and I am learning still. |
Co-authored-by: jim wang <[email protected]>
Co-authored-by: jim wang <[email protected]>
Co-authored-by: jim wang <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
…ously cmake would error with 'The target name hlslpp::hlslpp is reserved or not valid'. Updated vcpkg version database
Usage test pass with following triplet:
|
Co-authored-by: Kai Pastor <[email protected]>
Co-authored-by: Kai Pastor <[email protected]>
find_package
calls are REQUIRED, are satisfied byvcpkg.json
's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.json
matches what upstream says.vcpkg.json
matches what upstream says../vcpkg x-add-version --all
and committing the result.