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

[sparrow] Add sparrow #37221

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

[sparrow] Add sparrow #37221

wants to merge 11 commits into from

Conversation

jjerphan
Copy link
Contributor

@jjerphan jjerphan commented Mar 7, 2024

  • 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 adding-usage for context.
  • 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.
@jjerphan jjerphan force-pushed the sparrow branch 7 times, most recently from acd27e1 to 9effd34 Compare March 7, 2024 15:06
@MonicaLiu0311 MonicaLiu0311 changed the title Add sparrow Mar 8, 2024
@MonicaLiu0311 MonicaLiu0311 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Mar 8, 2024
@jjerphan jjerphan force-pushed the sparrow branch 3 times, most recently from 2d9463c to 90178d3 Compare March 14, 2024 09:28
@jjerphan jjerphan marked this pull request as ready for review March 14, 2024 09:36
@jjerphan jjerphan force-pushed the sparrow branch 3 times, most recently from 2b3c442 to 3b4c929 Compare March 14, 2024 09:54
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
@MonicaLiu0311
Copy link
Contributor

The usage test passed on x64-windows (header files found):

sparrow is header-only and can be used from CMake via:

    find_path(SPARROW_INCLUDE_DIRS "sparrow/buffer.hpp")
    target_include_directories(main PRIVATE ${SPARROW_INCLUDE_DIRS})
MonicaLiu0311
MonicaLiu0311 previously approved these changes Mar 15, 2024
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Mar 15, 2024
@jjerphan
Copy link
Contributor Author

Thank you, @MonicaLiu0311.

@BillyONeal
Copy link
Member

  • 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.

This does not seem to be correct. This project is not associated with sparrow on repology, and first search results point to https://github.com/ropas/sparrow , not https://github.com/xtensor-stack/sparrow .

Would you accept xtensor-stack-sparrow ?

@BillyONeal BillyONeal removed the info:reviewed Pull Request changes follow basic guidelines label Mar 16, 2024
@BillyONeal BillyONeal marked this pull request as draft March 16, 2024 01:18
@jjerphan
Copy link
Contributor Author

x-sparrow would work for us as the ownership might be shared beyond the xtensor-stack's organization.

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
@jjerphan jjerphan marked this pull request as ready for review March 18, 2024 09:11
@BillyONeal BillyONeal removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 28, 2024
@BillyONeal BillyONeal marked this pull request as draft March 28, 2024 21:49
@JohanMabille
Copy link

The thing is that sparrow will be transfered to the man-group org when it's more stable. Would man-group-sparrow be acceptable?

@BillyONeal
Copy link
Member

The thing is that sparrow will be transfered to the man-group org when it's more stable. Would man-group-sparrow be acceptable?

Can't we just rename the port if/when that happens?

@jjerphan
Copy link
Contributor Author

jjerphan commented Apr 9, 2024

Does renaming the port makes the old name available again? If so, could name this port xtensor-stack-sparrow for now, and rename it when xtensor-stack/sparrow becomes man-group/sparrow? Alternatively, let's keep this PR open to check we don't break the build until the transfer.

@BillyONeal
Copy link
Member

Does renaming the port makes the old name available again?

No, in general people would have to update their references. But they'd have to do that referencing the github repo in the first place.

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
@jjerphan jjerphan marked this pull request as ready for review June 13, 2024 08:43
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
As `date` does not supports it.

See:
https://github.com/microsoft/vcpkg/blob/master/ports/date/vcpkg.json#L21

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
@jjerphan
Copy link
Contributor Author

Hello @BillyONeal,

The project has been transferred to another organization, and this PR is now ready for another review. 🙂

@BillyONeal
Copy link
Member

The naming concern remains. Would you accept man-group-sparrow to follow the [github organization]-[repo] convention?

@BillyONeal BillyONeal marked this pull request as draft June 18, 2024 20:28
@jjerphan jjerphan changed the title [cpp-sparrow] Add cpp-sparrow Jun 19, 2024
@jjerphan
Copy link
Contributor Author

Can we actually reconsider sparrow as a name since https://github.com/ropas/sparrow is a small, unmaintained OCAML project?

@SylvainCorlay
Copy link

I would love this to be named sparrow. It would save us the trouble of having to tell people to use a different name on vcpkg.

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
@jjerphan jjerphan changed the title [man-group-sparrow] Add man-group-sparrow Jun 19, 2024
@jjerphan jjerphan marked this pull request as ready for review June 20, 2024 06:35
@BillyONeal
Copy link
Member

BillyONeal commented Jun 27, 2024

The problem with the name sparrow is again, not in repology, and search hits lead to

  • ropas/sparrow
  • Sparrow-lang/sparrow

which are not this.

@BillyONeal BillyONeal added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jun 27, 2024
@BillyONeal
Copy link
Member

@AugP
@ras0219-msft
@data-queue
@JavierMatosD
and I discussed this today.

We affirm what we said back in March, there's too much potential for confusion and we want the name to be man-group-sparrow.

@BillyONeal BillyONeal removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jun 27, 2024
@BillyONeal BillyONeal marked this pull request as draft June 27, 2024 22:26
@MonicaLiu0311
Copy link
Contributor

Ping @jjerphan

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!
6 participants