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.

Sign up for GitHub

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

Jump to bottom

[vcpkg] Two bug fixes and some improvements in buildsystems/msbuild #13755

Merged
merged 15 commits into from
Nov 19, 2020
Merged

[vcpkg] Two bug fixes and some improvements in buildsystems/msbuild #13755

merged 15 commits into from
Nov 19, 2020

Conversation

FrankHeimes
Copy link
Contributor

Applied changes as described in #13753

  • Fixed: Empty fields in Vcpkg property sheet
  • Fixed: Missing -static suffix in VcpkgCurrentInstalledDir
  • Removed redundant property definitions
  • Various small improvements

@PhoebeHui PhoebeHui changed the title Dev/frankheimes/13753 [vcpkg] Two bug fixes and some improvements in buildsystems/msbuild Sep 27, 2020
@PhoebeHui PhoebeHui added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Sep 27, 2020
@JackBoosY JackBoosY requested a review from dan-shaw September 27, 2020 03:01
@FrankHeimes
Copy link
Contributor Author

FrankHeimes commented Sep 27, 2020 •
edited
Loading

Overriding $(VcpkgTriplet) in VcpkgTriplet.vcxproj doesn't work anymore. Working on fixing it ...

@JackBoosY
Copy link
Contributor

Please merge to master branch.

@FrankHeimes
Copy link
Contributor Author

As I see it, I'm done with my changes and fixes and humbly awaiting your code review remarks.

@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@BillyONeal BillyONeal self-assigned this Nov 5, 2020
strega-nil
strega-nil reviewed Nov 18, 2020
View reviewed changes
Copy link
Contributor

@strega-nil strega-nil left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks like the only thing to me, thanks @FrankHeimes :)

Could you also merge with master?

@FrankHeimes
Code Review Correction
7239235 
Removed stray double quote reported by @strega-nil
@JackBoosY
Copy link
Contributor

Please solve the file conflict,

Thanks.

@JackBoosY JackBoosY requested a review from strega-nil November 19, 2020 08:55
@strega-nil
Copy link
Contributor

This LGTM; however, I don't really understand MSBuild, so I'm just going to try it out and see if it works.

@strega-nil
Copy link
Contributor

Looks like it works, although I think there's one thing I'd like to change.

@strega-nil strega-nil merged commit e126e3e into microsoft:master Nov 19, 2020
This was referenced Nov 23, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JackBoosY JackBoosY JackBoosY approved these changes

@dan-shaw dan-shaw Awaiting requested review from dan-shaw

@strega-nil strega-nil Awaiting requested review from strega-nil

Assignees

@BillyONeal BillyONeal

@PhoebeHui PhoebeHui

@JackBoosY JackBoosY

Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@FrankHeimes @JackBoosY @strega-nil @BillyONeal @PhoebeHui