[exiv2] Make add nls feature #18964
[exiv2] Make add nls feature #18964
Conversation
| @@ -53,6 +53,11 @@ vcpkg_install_cmake() | |||
| vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/exiv2) | |||
| vcpkg_fixup_pkgconfig() | |||
|
|
|||
| if("nls" IN_LIST FEATURES) | |||
| set(EXIV2_ENABLE_NLS ON) | |||
There was a problem hiding this comment.
This doesn't seem connected to anything? I think you need to move it up and change
-DEXIV2_ENABLE_NLS=OFF
to
-DEXIV2_ENABLE_NLS=${EXIV2_ENABLE_NLS}
There was a problem hiding this comment.
Actually I note that this if is already the behavior of the vcpkg_check_features above.
There was a problem hiding this comment.
To make it available for configure file:
https://github.com/microsoft/vcpkg/pull/18964/files#diff-1bdf222c873035319124c25b484ba3840ebc8adb43ea4731e43e4627dcf39ac3L56
Being used in here:
https://github.com/microsoft/vcpkg/pull/18964/files#diff-2f0b76215a2d1a149da5bfea386e4e22c09569045184d1b9fbd363fd4b784e5aR5
Is that not the correct approach?
There was a problem hiding this comment.
Ah, I see. That makes sense, thanks! I also missed that there is a ${FEATURE_OPTIONS} for the other features so I was mistaken about that too -- thanks GitHub for hiding important context in your code review tool :(
There was a problem hiding this comment.
thanks GitHub for hiding important context in your code review tool :(
deciding what's relevant to get the best signal to noise ratio sounds like a non-trivial job ...
but while we are at it, I think this repository could benefit from some ci tooling to detect things like incorrect or missing usage of x-add-version and ideally even some integrated bot magic for fixing these issues...
There was a problem hiding this comment.
deciding what's relevant to get the best signal to noise ratio sounds like a non-trivial job ...
The Azure DevOps code review tool just always shows you the whole file, it's never confusing. But there's no way to configure GitHub to do that :(
| @@ -53,6 +53,11 @@ vcpkg_install_cmake() | |||
| vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/exiv2) | |||
| vcpkg_fixup_pkgconfig() | |||
|
|
|||
| if("nls" IN_LIST FEATURES) | |||
| set(EXIV2_ENABLE_NLS ON) | |||
There was a problem hiding this comment.
Actually I note that this if is already the behavior of the vcpkg_check_features above.
JonLiu1993
added
the
category:port-feature
Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>
| @@ -53,6 +53,11 @@ vcpkg_install_cmake() | |||
| vcpkg_fixup_cmake_targets(CONFIG_PATH lib/cmake/exiv2) | |||
| vcpkg_fixup_pkgconfig() | |||
|
|
|||
| if("nls" IN_LIST FEATURES) | |||
| set(EXIV2_ENABLE_NLS ON) | |||
There was a problem hiding this comment.
Ah, I see. That makes sense, thanks! I also missed that there is a ${FEATURE_OPTIONS} for the other features so I was mistaken about that too -- thanks GitHub for hiding important context in your code review tool :(
|
Thanks for your contribution! |
Describe the pull request