8000 Fix #154 that broke task.hpp by using optional without adding #include by dok-net · Pull Request #156 · jbaldwin/libcoro · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix #154 that broke task.hpp by using optional without adding #include #156

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

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

dok-net
Copy link
Contributor
@dok-net dok-net commented Jun 21, 2023

@jbaldwin Please check that the README get regenerated, it didn't last time, and I have disabled that commit hook because it's not working here with my setup yet.

@dok-net dok-net marked this pull request as ready for review June 21, 2023 05:54
@jbaldwin
Copy link
Owner

Ok, probably I will make a standalone commit to update it then since it's out of date at the moment.

I assume this fix is for Arduino since all the CI pipelines passed?

@jbaldwin jbaldwin merged commit 214bb11 into jbaldwin:main Jun 21, 2023
@dok-net dok-net deleted the fix_154 branch June 21, 2023 18:32
@dok-net
Copy link
Contributor Author
dok-net commented Jun 21, 2023

I assume this fix is for Arduino since all the CI pipelines passed?

This reveals that it might be helpful to run CI also with all options disabled. It didn't occur exactly with Arduino - it would have been noticed there, too, to be sure - but in the CMake based use as a library module for Zephyr OS applications.

The upcoming Espressif IDF 5.1 and the Arduino core for the ESP32 family uses a recent GCC such that libcoro now works there, too, not only on the ESP8266. From what I read, CMake is or will be the default build system for their IDF.

@jbaldwin
Copy link
Owner

Aha! That makes perfect sense. Would you mind opening a bug to add that path to CI?

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.

2 participants
0