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

[scripts-audit] vcpkg_extract_source_archive #18517

Closed

Conversation

strega-nil
Copy link
Contributor

This PR audits vcpkg_extract_source_archive and vcpkg_extract_source_archive_ex to follow in line with issue #17691.

Additionally, it changes the "default function" back to vcpkg_extract_source_archive.

This is quite a sizeable change, so it needs serious CR from the team.

@PhoebeHui PhoebeHui added category:scripts-audit Part of the scripts audit effort. info:internal This PR or Issue was filed by the vcpkg team. info:world-rebuild labels Jun 18, 2021
@strega-nil-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

Maybe we need to talk as a team about this, but I don't think we should be deprecating things that our catalog does. The exercise of fixing the catalog to comply with the deprecated thing also helps the team better understand what the potential impact of a deprecation is.

scripts/cmake/vcpkg_extract_source_archive.cmake Outdated Show resolved Hide resolved
@ras0219-msft
Copy link
Contributor

Discussion offline: we should avoid adding a third overload that must be maintained forever -- instead any improvements should go into the existing preferred overload and we should simply make vcpkg_extract_source_archive and vcpkg_extract_source_archive_ex both support the same set of arguments.

@strega-nil-ms
Copy link
Contributor

Closed for rollup PR #18838

strega-nil-ms pushed a commit to strega-nil/vcpkg that referenced this pull request Jul 13, 2021
[scripts-audit] vcpkg_extract_source_archive
BillyONeal pushed a commit that referenced this pull request Jul 14, 2021
* [rollup:2021-07-06 1/8] PR #18272 (@strega-nil)

[scripts-audit] vcpkg_from_*

* [rollup:2021-07-06 2/8] PR #18319 (@strega-nil)

[scripts-audit] add guidelines for cmake

* [rollup 2021-07-06 3/8] PR #18410 (@mheyman)

[vcpkg-cmake-config] documentation fix

* [rollup:2021-07-06 4/8] PR #18488 (@strega-nil)

[scripts-audit] vcpkg_execute_*

* [rollup:2021-07-06 5/8] PR #18517 (@strega-nil)

[scripts-audit] vcpkg_extract_source_archive

* [rollup:2021-07-06 6/8] PR #18674 (@NancyLi1013)

[vcpkg doc] Update examples

* [rollup:2021-07-06 7/8] PR #18695 (@JackBoosY)

[vcpkg] Update the minimum version of vcpkg

* [rollup:2021-07-06 8/8] PR #18758 (@ras0219-msft)

[vcpkg_from_git] Fix error if downloads folder does not exist

* build docs!

* fix bond:*-windows

* fix nmap

Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
Co-authored-by: Michael Heyman <Michael.Heyman@jhuapl.edu>
Co-authored-by: NancyLi1013 <lirui09@beyondsoft.com>
Co-authored-by: JackBoosY <yuzaiyang@beyondsoft.com>
Co-authored-by: Robert Schumacher <ras0219@outlook.com>
@strega-nil strega-nil deleted the vcpkg_extract_source_archive branch July 16, 2021 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:scripts-audit Part of the scripts audit effort. info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants