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

[nanovg] Add dependency port stb #8813

Merged
merged 5 commits into from
Jan 6, 2020
Merged

[nanovg] Add dependency port stb #8813

merged 5 commits into from
Jan 6, 2020

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Oct 30, 2019 •
edited by PhoebeHui
Loading

Related issue #8809

Changes:

  1. Add dependency port stb to nanovg
  2. It doesn't need to install stb headers when build nanovg, so remove the steps.
  3. nanovg does not support uwp.

Note: this port does not contain any feature.

@JackBoosY JackBoosY added the info:internal This PR or Issue was filed by the vcpkg team. label Oct 30, 2019
@JackBoosY
Copy link
Contributor Author

@xarthurx Could you help me to test it?

Thanks.

@xarthurx
Copy link
Contributor

xarthurx commented Oct 30, 2019 •
edited
Loading

@JackBoosY
If testing means to uninstall nanovg and stb, and then install nanovg, the stb lib is installed as a dependency.

Everything works fine.

Thank you for the quick response.

@JackBoosY JackBoosY marked this pull request as ready for review October 30, 2019 11:46
@JackBoosY JackBoosY mentioned this pull request Oct 31, 2019
Closed
@xarthurx
Copy link
Contributor

xarthurx commented Nov 5, 2019 •
edited
Loading

Seems have some issue?

@JackBoosY
Copy link
Contributor Author

JackBoosY commented Nov 5, 2019 •
edited
Loading

@xarthurx Nop, This port does not support uwp, and we need to install autoconf / automake on the OS X CI machine.

@xarthurx
Copy link
Contributor

@PhoebeHui Hi, sorry to bother you, but would you please review this?
I'm waiting for this PR so that @JackBoosY can start fixing the libigl PR...

@JackBoosY
Copy link
Contributor Author

@xarthurx We need to wait to fix the CI issue.

@PhoebeHui
Copy link
Contributor

@xarthurx, thanks for reminding me, the changes looks good, I approved it, however, we are waiting for CI back online, after CI test pass, this PR should ready to merge.

@JackBoosY
Copy link
Contributor Author

/azp run

@JackBoosY
Copy link
Contributor Author

@dan-shaw This PR is ready to merge.

@PhoebeHui PhoebeHui changed the title [nanovg]Add depends port stb. Do not install stb headers. [nanovg] Add dependency port stb Nov 26, 2019
@grdowns grdowns self-assigned this Dec 3, 2019
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Dec 3, 2019
@dan-shaw
Copy link
Contributor

dan-shaw commented Dec 6, 2019

LGTM although the CI is down again, and we trying to fix it as soon as we can. See #9203 #9216

@xarthurx
Copy link
Contributor

How is this merge going?

@dan-shaw dan-shaw merged commit 0c481e1 into microsoft:master Jan 6, 2020
@cenit cenit mentioned this pull request Jan 6, 2020
Closed
@JackBoosY JackBoosY deleted the dev/jack/8809 branch January 7, 2020 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dan-shaw dan-shaw dan-shaw approved these changes

@PhoebeHui PhoebeHui PhoebeHui approved these changes

Assignees

@dan-shaw dan-shaw

Labels
info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JackBoosY @xarthurx @PhoebeHui @dan-shaw @grdowns @ras0219-msft