[vcpkg] Merge the vcpkg metadata uploader into the vcpkg binary#13421
[vcpkg] Merge the vcpkg metadata uploader into the vcpkg binary#13421BillyONeal merged 11 commits intomicrosoft:masterfrom
Conversation
…hat we need sign only one binary.
strega-nil
left a comment
There was a problem hiding this comment.
I would prefer to see the ifdef solution rather than the REMOVE_ITEM solution, because I think it's cleaner.
| memset(&startup_info, 0, sizeof(STARTUPINFOW)); | ||
| startup_info.cb = sizeof(STARTUPINFOW); | ||
| startup_info.dwFlags = STARTF_USESHOWWINDOW; | ||
| startup_info.wShowWindow = SW_HIDE; |
There was a problem hiding this comment.
Do we always want these flags?
There was a problem hiding this comment.
I think we do but the bits could be renamed to be clearer, will fix.
There was a problem hiding this comment.
Does my rename resolve your concern?
| Commands::Version::VersionCommand version{}; | ||
| } | ||
| template<class CommandListT, size_t ExpectedCount> | ||
| void check_all_commands(const CommandListT& actual_commands, const char* const (&expected_commands)[ExpectedCount]) |
There was a problem hiding this comment.
Suggestion: take std::initializer_list<StringLiteral> as the second argument.
There was a problem hiding this comment.
I don't think that's an improvement: initializer_list exists to be the thing that controls constructor overload resolution; using it for any other purpose just encourages use after free bugs when people treat it like a container instead of a pointer to a temporary array.
| memset(&startup_info, 0, sizeof(STARTUPINFOW)); | ||
| startup_info.cb = sizeof(STARTUPINFOW); | ||
| startup_info.dwFlags = STARTF_USESHOWWINDOW; | ||
| startup_info.wShowWindow = SW_HIDE; |
…, so that we need sign only one binary.