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.

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

foxglove-websocket: add version 0.0.1 #8662

Merged

Conversation

jtbandes
Copy link
Contributor

@jtbandes jtbandes commented Jan 7, 2022

Hello 👋

I'm the author of this package, foxglove-websocket, which is part of a larger project at foxglove/ws-protocol (read more here). We would like the C++ server library to be available on ConanCenter. This is the first version of the package. It's currently a header-only library, with dependencies on nlohmann_json and websocketpp.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.
    • Built in a conanio/clang10 Docker container using -s compiler.cppstd=17 -s compiler.libcxx=libc++.

@CLAassistant
Copy link

CLAassistant commented Jan 7, 2022

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jtbandes jtbandes changed the title Add foxglove-websocket/0.0.1 foxglove-websocket: add version 0.0.1 Jan 7, 2022
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

Looks really freaking good! Just two small details to keep it inline with CCI

Having used websocketpp in the past this looks like a welcomed improvement!

@conan-center-bot

This comment has been minimized.

@jtbandes
Copy link
Contributor Author

jtbandes commented Jan 12, 2022

Hm, I wonder if this new error is due to the tools.check_min_cppstd not running? Any advice @prince-chrismc?

@SSE4
Copy link
Contributor

SSE4 commented Jan 12, 2022

Hm, I wonder if this new error is due to the tools.check_min_cppstd not running? Any advice @prince-chrismc?

yes, I think so. it tries to compile with old standard, but as I can see, it needs C++17 at very least, right?

@jtbandes
Copy link
Contributor Author

Yes, that was the purpose of adding check_min_cppstd. How else can I achieve this?

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jtbandes
Copy link
Contributor Author

@prince-chrismc If you have some advice that would be appreciated! I'm stuck...it really seems like check_min_cppstd is necessary since I need to ensure c++17 is used (both language and standard library version).

conan_basic_setup()

add_executable(test_package test_package.cpp)
target_link_libraries(test_package ${CONAN_LIBS})
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking quickly you'll need to configure C++17 here ... here's an example

set_target_properties(example PROPERTIES CXX_STANDARD 17)

I'll take a better look tomorrow of theres anything else I can find

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that helped for setting the language standard but still running into problems with the standard library version.

@conan-center-bot

This comment has been minimized.

@prince-chrismc
Copy link
Contributor

Yuck. My system was out of date... Sadly everything checks out on my setup 🤔

CXX=g++-7 CC=gcc-7 conan create all 0.0.1@ -s compiler.version=7 works just fine
I am running a newer gcc 7.5 instead of 7.2 in the docker images

Maybe try bumping the min compiler to 8?

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Jan 15, 2022
Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

🚀

raise ConanInvalidConfiguration("Compiler version is not supported, c++17 support is required")

def configure(self):
self.options["websocketpp"].asio = "standalone"
Copy link
Member

Choose a reason for hiding this comment

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

Okay. The default value is "boost", without it, this package would be invalid.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot
Copy link
Collaborator

All green in build 15 (97ed9ca5a43541861c9f956599e8c1fadd11277c):

  • foxglove-websocket/0.0.1@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit ea7ecb7 into conan-io:master Jan 21, 2022
@jtbandes jtbandes deleted the jacob/add-foxglove-websocket branch January 21, 2022 17:36
@jtbandes jtbandes mentioned this pull request Apr 1, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants