Skip to content

Conversation

gnaggnoyil
Copy link
Contributor

Describe the pull request

  • What does your PR fix? Fixes [New Port Request] hffix #13432

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    • All triplets should be supported. No, CI baseline isn't updated.
  • Does your PR follow the maintainer guide?

    • Yes

Comment on lines 3 to 7
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO jamesdbrock/hffix
REF v1.0.0
SHA512 0043b789e6ffdc32eaf2736a8621dd7fd54e1a16aae33bb1d5f642da1b04d150ed42d8f9ddd046013242164854d9091540452153f09459d05f9bf4a186c7b860
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO jamesdbrock/hffix
REF v1.0.0
SHA512 0043b789e6ffdc32eaf2736a8621dd7fd54e1a16aae33bb1d5f642da1b04d150ed42d8f9ddd046013242164854d9091540452153f09459d05f9bf4a186c7b860
vcpkg_from_github(
OUT_SOURCE_PATH SOURCE_PATH
REPO jamesdbrock/hffix
REF v1.0.0
SHA512 0043b789e6ffdc32eaf2736a8621dd7fd54e1a16aae33bb1d5f642da1b04d150ed42d8f9ddd046013242164854d9091540452153f09459d05f9bf4a186c7b860
HEAD_REF master

Comment on lines 12 to 14
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA
vcpkg_configure_cmake(
SOURCE_PATH ${SOURCE_PATH}
PREFER_NINJA


include(GNUInstallDirs)

add_library(hffix-header INTERFACE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_library(hffix-header INTERFACE)
add_library(hffix INTERFACE)

In my opinion, it might be better to rename it as hffix.

include(GNUInstallDirs)

add_library(hffix-header INTERFACE)
target_include_directories(hffix-header INTERFACE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
target_include_directories(hffix-header INTERFACE
target_include_directories(hffix INTERFACE

)

install(
TARGETS hffix-header
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TARGETS hffix-header
TARGETS hffix

@NancyLi1013 NancyLi1013 added category:new-port The issue is requesting a new library to be added; consider making a PR! requires:author-response labels Sep 10, 2020
@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Sep 11, 2020
@ras0219-msft ras0219-msft merged commit 99a74a7 into microsoft:master Sep 11, 2020
@ras0219-msft
Copy link
Collaborator

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] hffix
3 participants