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

[vcpkg] improve s390x support #13386

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

lebdron
Copy link
Contributor

@lebdron lebdron commented Sep 7, 2020

Improve s390x support based on #11880

  • Set vcpkgUseSystem in bootstrap.sh
  • Check VCPKG_FORCE_SYSTEM_BINARIES
  • Define default triplet

@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Sep 7, 2020
@PhoebeHui
Copy link
Contributor

@lebdron, could you format the C++ file?

The formatting of the C++ files didn't match our expectation.
See github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#vcpkg-internal-code for solution.
File list:
toolsrc/src/vcpkg.cpp
clang-format should produce the following diff:
diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@lebdron
Copy link
Contributor Author

lebdron commented Sep 7, 2020

@PhoebeHui Done, thanks! I would also suggest to specify in documentation which version of clang-format is used for CI, because clang-format-7 and clang-format-10 may have different outputs, for example. I see that "Install llvm clang-format" link points to version 10.0, but that is not that obvious in my opinion.

@PhoebeHui
Copy link
Contributor

@lebdron, clang-format-7 and clang-format-10 should be okay, we defined the format in https://github.com/microsoft/vcpkg/blob/master/toolsrc/.clang-format.

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR!

@dan-shaw, could you help furhter review?

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Sep 8, 2020
@ras0219-msft ras0219-msft merged commit 645f316 into microsoft:master Sep 9, 2020
@ras0219-msft
Copy link
Contributor

LGTM, really cool to see further improvements here :)

@lebdron lebdron deleted the s390-improvements branch September 10, 2020 08:15
strega-nil pushed a commit to strega-nil/vcpkg that referenced this pull request May 5, 2021
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants