-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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] Use more forward declarations rather than definitions #13623
[vcpkg] Use more forward declarations rather than definitions #13623
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this @cngzhnp :)
The CI failures look legitimate |
Yes, I did not realize that compilation of tests is failed. Is there any batch file to run it or just run cmake on vcpkg-test? |
Right, you can see the exact commands we run in the ymls in |
It does indeed look like configuring CMake with tests on: Line 12 in 2a6442c
rather than trying to use the msbuild based bootstrap would show you these failures:
Billy3 |
@BillyONeal Thanks, at least I could generate VS19 solution from CMakeLists.txt and saw that update.cpp was failed during compilation. It seems that it is fixed right now but let's see what CI brings to me :) |
It seems like for most of these we should extract |
To clarify, I'm saying we should apply Nicole's feedback in the previous PR ( #12836 (comment) ) here too |
@BillyONeal It is OK from my perspective. I've mentioned in PR description if new files need to be created, I can do it in this PR. Otherwise, we can close this, create new one to refer this and make changes in there. What do you think? |
I would feel better if |
17781a9
to
8983270
Compare
8983270
to
d1f1c8f
Compare
Could you please review my changes again @BillyONeal @strega-nil? |
d1f1c8f
to
0a40329
Compare
/azp run |
No pipelines are associated with this pull request. |
Thanks for your contribution! |
…oft#13623) Co-authored-by: Billy Robert O'Neal III <[email protected]>
I had done before in #12836 and these are new fixes for other files as well. I try to use forward declaration files as much as possible. Please give feedback about if new files need to be created.