Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[pffft] new port #20624

Merged
merged 2 commits into from
Oct 12, 2021
Merged

[pffft] new port #20624

merged 2 commits into from
Oct 12, 2021

Conversation

chausner
Copy link
Contributor

@chausner chausner commented Oct 9, 2021

Describe the pull request

  • What does your PR fix?

    Adds new port for pffft.

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

    All, Yes

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

@PhoebeHui PhoebeHui self-assigned this Oct 11, 2021
@PhoebeHui PhoebeHui added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Oct 11, 2021
PhoebeHui
PhoebeHui approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Oct 11, 2021
BillyONeal
BillyONeal reviewed Oct 12, 2021
View reviewed changes
ports/pffft/CMakeLists.txt
DESTINATION share/pffft
)

if(NOT DISABLE_INSTALL_HEADERS)
Copy link
Member

Choose a reason for hiding this comment

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

If this is always true and we authored this CMakeLists.txt, why have the option at all?

Copy link
Contributor

@PhoebeHui PhoebeHui Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We don't install the headers for debug configuration, see ' OPTIONS_DEBUG' in portfile.cmake. Only installed the headers for release configuration.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I missed that, thanks!

@BillyONeal BillyONeal added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Oct 12, 2021
@PhoebeHui PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Oct 12, 2021
@BillyONeal BillyONeal merged commit 677eae7 into microsoft:master Oct 12, 2021
@chausner chausner deleted the pffft branch October 12, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BillyONeal BillyONeal BillyONeal left review comments

@PhoebeHui PhoebeHui PhoebeHui approved these changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees

@PhoebeHui PhoebeHui

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chausner @BillyONeal @PhoebeHui