Skip to content

refactor channel push and pop return values#79

Merged
wokron merged 10 commits into
masterfrom
new-channel-api
Mar 30, 2026
Merged

refactor channel push and pop return values#79
wokron merged 10 commits into
masterfrom
new-channel-api

Conversation

@wokron
Copy link
Copy Markdown
Member

@wokron wokron commented Mar 28, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors condy::Channel to use errno-style integer return codes for both synchronous (try_push/try_pop) and coroutine (push/pop) operations, and updates tests/docs accordingly while introducing a deprecated legacy channel API for the previous behavior.

Changes:

  • Change Channel push/pop APIs to return int / (int, T) results (e.g., 0, -EAGAIN, -EPIPE, -ECANCELED) instead of bool/optional/exceptions.
  • Update channel-related tests to match the new return types and error codes.
  • Add condy::legacy::Channel (deprecated) and update guide/benchmark code for the new API.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
include/condy/channel.hpp Refactors Channel API and awaiter completion to use errno-style results; includes legacy header.
include/condy/channel_legacy.hpp Adds deprecated legacy channel implementation preserving prior semantics.
tests/test_channel.cpp Updates channel unit tests to the new return conventions.
tests/test_async_operations.1.cpp Adjusts channel pop usage in async recvmsg multishot test to new return type.
tests/test_async_operations.2.cpp Adjusts channel push/pop usage in async read multishot tests to new return type.
tests/test_async_operations.3.cpp Adjusts channel push/pop usage in async recv multishot tests to new return type.
docs/guide.md Updates guide examples to new channel pop contract and closes channel in producer example.
benchmarks/channel.cpp Updates benchmark consumer to destructure new pop return type.
Comments suppressed due to low confidence (1)

tests/test_channel.cpp:370

  • This test still relies on item == 0 to detect a closed channel, but the refactor makes pop() return an explicit error code. Please check r == 0 for the first max_items pops, and then assert r == -EPIPE after push_close() (instead of relying on a default value) so the test matches the new API semantics.
    auto consumer = [&]() -> condy::Coro<void> {
        for (size_t i = 0; i < 2 * max_items; ++i) {
            auto [r, item] = co_await channel.pop();
            if (i < max_items) {
                REQUIRE(item == static_cast<int>(i + 1));
            } else {
                REQUIRE(item == 0); // Default indicates closed channel
            }
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_channel.cpp
Comment thread tests/test_channel.cpp
Comment thread tests/test_channel.cpp
Comment thread tests/test_async_operations.1.cpp
Comment thread tests/test_async_operations.3.cpp
Comment thread tests/test_channel.cpp Outdated
Comment thread tests/test_async_operations.2.cpp
Comment thread include/condy/channel.hpp
Comment thread docs/guide.md Outdated
Comment thread tests/test_channel.cpp
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/condy/channel.hpp Outdated
Comment thread tests/test_channel.cpp
Comment thread docs/guide.md
@wokron wokron marked this pull request as ready for review March 30, 2026 12:02
@wokron wokron merged commit f703e05 into master Mar 30, 2026
11 checks passed
@wokron wokron deleted the new-channel-api branch March 30, 2026 12:14
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