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.

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

[cglm] New port #29024

Merged
merged 6 commits into from
Jan 23, 2023
Merged

[cglm] New port #29024

merged 6 commits into from
Jan 23, 2023

Conversation

chausner
Copy link
Contributor

Describe the pull request

  • What does your PR fix?

    Adds new port for cglm

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

    Note sure yet, 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

github-actions[bot]
github-actions bot previously approved these changes Jan 17, 2023
@Adela0814 Adela0814 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jan 18, 2023
@chausner chausner marked this pull request as ready for review January 18, 2023 18:04
github-actions[bot]
github-actions bot previously approved these changes Jan 18, 2023
@@ -0,0 +1,18 @@
{
"name": "cglm",
"version-semver": "0.8.8",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adela0814
Adela0814 previously approved these changes Jan 19, 2023
@Adela0814 Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Jan 19, 2023
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

  • Changes comply with the maintainer guide
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See docs/examples/adding-an-explicit-usage.md for context.

Can you please confirm that the following looks correct? (The upstream instructions use add_subdirectory :(

cglm provides CMake targets:

    find_package(cglm CONFIG REQUIRED)
    target_link_libraries(main PRIVATE cglm::cglm)
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@chausner
Copy link
Contributor Author

Can you please confirm that the following looks correct? (The upstream instructions use add_subdirectory :(

Yes that should be correct but I have not tested it. They do write in their README (albeit without an example):

or you can use find_package to configure cglm

In their tests, they link with

target_link_libraries(${TEST_MAIN} PRIVATE cglm)

but I assume this is equivalent to linking with cglm::cglm.

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Added usage. Will merge as soon as build comes back. Thank you!

@BillyONeal BillyONeal merged commit 7632bef into microsoft:master Jan 23, 2023
@chausner chausner deleted the cglm-0.8.8 branch January 23, 2023 23:31
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.

3 participants