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

[zlmediakit] update zlmediakit to 20240330.0 #37654

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

xia-chu
Copy link
Contributor

@xia-chu xia-chu commented Mar 23, 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.

@FrankXie05 FrankXie05 self-assigned this Mar 25, 2024
@FrankXie05 FrankXie05 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Mar 25, 2024
@BillyONeal
Copy link
Member

BillyONeal commented Mar 28, 2024

Can you explain why you're adding a default feature like this? Other than that everything seems reasonable to me...

@FrankXie05 FrankXie05 added the category:port-update The issue is with a library, which is requesting update new revision label Mar 29, 2024
@xia-chu
Copy link
Contributor Author

xia-chu commented Mar 29, 2024

Can you explain why you're adding a default feature like this? Other than that everything seems reasonable to me...

Many newcomers who are just getting started don't know how to use vcpkg and zlmediakit. Enabling all features by default can reduce their mental burden, and there is no cost to doing so.

FrankXie05
FrankXie05 previously approved these changes Mar 29, 2024
@FrankXie05 FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Mar 29, 2024
@BillyONeal
Copy link
Member

The problem is that getting default features to be off can be difficult for customers to do, for example: #35694

In general, we try to avoid making a feature default unless the resulting library is ~broken without one. For example, a libarchive with no archive backends is worthless.

Enabling all features by default can reduce their mental burden, and there is no cost to doing so.

There are costs to doing so because this adds a dependency on that feature for callers who might not be expecting this. Is there a particular thing about zlmediakit which makes it not worthwhile for the vast majority of users with this feature off?

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:reviewed Pull Request changes follow basic guidelines labels Mar 30, 2024
@BillyONeal
Copy link
Member

Tagging vcpkg-team-review because we need to discuss how we describe how default features are to be used

@xia-chu
Copy link
Contributor Author

xia-chu commented Mar 30, 2024

The problem is that getting default features to be off can be difficult for customers to do, for example: #35694

OK, i had disabled sctp default, and pushed new commits, thanks!

@xia-chu xia-chu changed the title [zlmediakit] update zlmediakit's code and enable sctp default [zlmediakit] update zlmediakit to 20240330.0 Mar 30, 2024
@BillyONeal BillyONeal merged commit a164ace into microsoft:master Apr 1, 2024
16 checks passed
@BillyONeal
Copy link
Member

Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-update The issue is with a library, which is requesting update new revision requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants