Skip to content

Conversation

gjasny
Copy link
Contributor

@gjasny gjasny commented Mar 28, 2020

Update civetweb from snapshot release to regular 1.12 release. Tested locally on MacOS against prometheus-cpp.

Copy link
Contributor

@Cheney-W Cheney-W left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

  1. include(vcpkg_common_functions) is no longer needed, could you please remove this line?
  2. Could you please modify
    if(VCPKG_CMAKE_SYSTEM_NAME STREQUAL "WindowsStore")
    message(FATAL_ERROR "${PORT} does not currently support UWP")
    endif()
    as below:
    vcpkg_fail_port_install(MESSAGE "${PORT} does not currently support UWP" ON_TARGET "UWP")
  3. Could you please modify
    configure_file(${SOURCE_PATH}/LICENSE.md ${CURRENT_PACKAGES_DIR}/share/civetweb/copyright COPYONLY)
    as below:
    file(INSTALL ${SOURCE_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)

@gjasny
Copy link
Contributor Author

gjasny commented Mar 30, 2020

Thanks for the review. I addressed all of your comments and re-tested locally.

@Cheney-W
Copy link
Contributor

@gjasny
Could you please remove the below lines from vcpkg/scripts/ci.baseline.txt?
civetweb:x64-windows = skip
civetweb:x64-windows-static = skip
civetweb:x86-windows = skip

@gjasny
Copy link
Contributor Author

gjasny commented Mar 31, 2020

@Cheney-W done

@LilyWangL
Copy link
Contributor

@gjasny Thanks for your PR. I have tested this PR on Linux and MACOS, and it passed. Please remove the below lines from vcpkg/scripts/ci.baseline.txt.
civetweb:x64-linux = skip
civetweb:x64-osx = skip

@Cheney-W
Copy link
Contributor

@gjasny Could you address Lily's suggestion? Thanks!

@gjasny
Copy link
Contributor Author

gjasny commented Apr 29, 2020

Sorry, I must have missed the notification email.

@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed waiting for response labels Apr 30, 2020
@strega-nil
Copy link
Contributor

Alright, this looks great! Thanks @gjasny :)

@strega-nil strega-nil merged commit 43d664a into microsoft:master May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants