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

[ms-gsl] Update to v2.1.0, the "end of 2019 snapshot" #9624

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

Farwaykorse
Copy link
Contributor

@Farwaykorse Farwaykorse commented Jan 10, 2020

  • What does your PR fix?
    • [ms-gsl]
      • This updates the GSL to the versioned snapshot created at the end of 2019.
        Changes for v2.1.0.
        Didn't update to the current HEAD, because not all CI builds are succeeding on Linux.
        (The testsuite of the GSL is being migrating from Catch to GTest.)
      • Removing the deprecated include(vcpkg_common_functions).
    • [coroutine]
      Updated with a fix (add missing #include <cassert> luncliff/coroutine#45) required to build with recent versions of the GSL.
  • Which triplets are supported/not supported? Have you updated the CI baseline?
    No changes.

@PhoebeHui PhoebeHui changed the title [ms-gsl] update to v2.1.0, the "end of 2019 snapshot" [ms-gsl] Update to v2.1.0, the "end of 2019 snapshot" Jan 13, 2020
@PhoebeHui
Copy link
Contributor

@Farwaykorse, thanks for the PR!

The CI failed with following regression, is this expected? could you take a look?
[error] REGRESSION: coroutine:x64-windows
[error] REGRESSION: coroutine:x64-windows-static
[error] REGRESSION: coroutine:arm64-windows

@Farwaykorse
Copy link
Contributor Author

@PhoebeHui Thanks for checking. This is indeed unexpected.
I'll have to take a look at the problem with luncliff/coroutine.

@Farwaykorse
Copy link
Contributor Author

Farwaykorse commented Jan 13, 2020

Added the fix.
For a working history I changed the commit order for the fix (and update of coroutine) to come before the update of ms-gsl.
(Even though GitHub does still render them by their first creation date in this overview.)

@Farwaykorse Farwaykorse marked this pull request as ready for review January 13, 2020 12:44
@luncliff
Copy link
Contributor

@PhoebeHui @Farwaykorse Thanks for letting me know the issue :)
For the coroutine I will take a look and submit a PR for it soon.

@Farwaykorse
Copy link
Contributor Author

@luncliff coroutine should be fine with commit 0ebb574

A separate PR would normally be nice, but GitHub has no method to mark PR's as dependent on an other PR.

@ras0219-msft
Copy link
Contributor

/azp run

@ras0219-msft
Copy link
Contributor

All the CI errors look to be unrelated to this PR; no changes needed

@PhoebeHui
Copy link
Contributor

/azp run

@PhoebeHui
Copy link
Contributor

The CI failed with '##[error] REGRESSION: qt5-tools:x86-windows' it seems not related to this changes:

jom: C:\vsts\_work\3\s\buildtrees\qt5-tools\x86-windows-dbg\src\linguist\lupdate\Makefile.Debug [..\..\..\bin\lupdate.exe] Error 1104
jom: C:\vsts\_work\3\s\buildtrees\qt5-tools\x86-windows-dbg\src\linguist\lupdate\Makefile [debug] Error 2
jom: C:\vsts\_work\3\s\buildtrees\qt5-tools\x86-windows-dbg\src\linguist\Makefile [sub-lupdate-make_first] Error 2
C:\vsts\_work\3\s\buildtrees\qt5-tools\src\5.12.5-28cc1badc0\src\linguist\linguist\main.cpp(0): Note: No relevant classes found. No output generated.
jom: C:\vsts\_work\3\s\buildtrees\qt5-tools\x86-windows-dbg\src\Makefile [sub-linguist-make_first] Error 2
jom: C:\vsts\_work\3\s\buildtrees\qt5-tools\x86-windows-dbg\Makefile [sub-src-make_first] Error 2

@JackBoosY
Copy link
Contributor

Needs to rerun pipeline test after #9752 merged.

@PhoebeHui
Copy link
Contributor

The CI test pass, it seems no issue block this PR.

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jan 21, 2020
@vicroms vicroms merged commit fbdce55 into microsoft:master Jan 27, 2020
@vicroms
Copy link
Member

vicroms commented Jan 27, 2020

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants