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

[bw-tempdir] add new port #42342

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Conversation

bw-hro
Copy link
Contributor

@bw-hro bw-hro commented Nov 24, 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.

@Mengna-Li Mengna-Li added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Nov 25, 2024
Mengna-Li
Mengna-Li previously approved these changes Nov 25, 2024
@Mengna-Li
Copy link
Contributor

Usage tested pass on x64-windows.

@Mengna-Li Mengna-Li added the info:reviewed Pull Request changes follow basic guidelines label Nov 25, 2024
@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.

As far as I can see, this is not true. Would you be happy if the name of the port was bw-hro-tempdir?

@BillyONeal BillyONeal marked this pull request as draft November 25, 2024 21:04
@bw-hro
Copy link
Contributor Author

bw-hro commented Nov 25, 2024

Hi, thank you very much for your feedback and the value-able work on the vcpkg project. Preferred name would be "tempdir" and to be honest I can't see an issue with that. There are "tempdir" named packages from python and rust world, so I think "tempdir" for c++ ecosystem wouldn't do any harm? Or do I miss something? All this packages as well as @tempdir from Java attempt to provide the same feature of self cleaning temporary directories. So from my point of view "tempdir" would be the natural fit also for c++.

Alternatives I can think of would be in order of preference:

  • "tempdir-cpp" as this might be close to search strings (not sure if allowed according to maintainer guide
  • "bw-tempdir" as this would mirror the namespace bw::tempdir used in the project (but in general "bw" doesn't add any value)

Please advice if renaming is really required and in case it is, which version you would accept.

Thanks

@BillyONeal
Copy link
Member

There is no objection to your library being named tempdir. In general, we want names to be of the form <GithubOrganization>-<Library> unless the name has conclusively demonstrated that that is what people expect when they say vcpkg install <Library>. If, in the future, this becomes the defacto implementation of 'tempdir' for C++ programmers, which we would observe in repology and/or web search results and/or direct customer feedback, we can always create tempdir which just points to bw-hro-tempdir in the future.

Our concern is that once we have 'given away' a name, we can never change it to something else. That is, vcpkg install tempdir would have to remain pointing at your library, even if after a few years you decide to stop working on it and something else becomes the defacto provider of that name.

@bw-hro
Copy link
Contributor Author

bw-hro commented Nov 26, 2024

Ok, understand. Would you accept "bw-tempdir" as I like it more because it is shorter and resembles the namespace/include path #include <bw/tempdir/tempdir.hpp> while still having a creator related prefix?

@Mengna-Li Mengna-Li removed the info:reviewed Pull Request changes follow basic guidelines label Nov 26, 2024
@bw-hro
Copy link
Contributor Author

bw-hro commented Nov 26, 2024

Renamed the port from "tempdir" to "bw-tempdir" hope this is fine.

@bw-hro bw-hro marked this pull request as ready for review November 26, 2024 21:12
@BillyONeal BillyONeal changed the title [tempdir] add new port [bw-tempdir] add new port Nov 26, 2024
@BillyONeal
Copy link
Member

@ras0219-msft @vicroms and I think bw-tempdir is reasonable, thank you!

@BillyONeal BillyONeal merged commit be69702 into microsoft:master Nov 26, 2024
17 checks passed
@Mengna-Li Mengna-Li added the info:reviewed Pull Request changes follow basic guidelines label Nov 29, 2024
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