Skip to content

Add IoChannelCapTableEntry::getType().#6323

Open
kentonv wants to merge 2 commits intomainfrom
kenton/dynamic-ctx-exports
Open

Add IoChannelCapTableEntry::getType().#6323
kentonv wants to merge 2 commits intomainfrom
kenton/dynamic-ctx-exports

Conversation

@kentonv
Copy link
Member

@kentonv kentonv commented Mar 14, 2026

There's one place in the internal codebase that needs it.

There's one place in the internal codebase that needs it.
@kentonv kentonv requested a review from harrishancock March 14, 2026 02:09
@kentonv kentonv requested review from a team as code owners March 14, 2026 02:09
Co-authored-by: kentonv <kentonv@users.noreply.github.com>
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 14, 2026

The change is minimal and straightforward: a 2-line addition of a const getter for the private type field on IoChannelCapTableEntry. Let me verify there are no concerns:

  1. getType() is const-correct -- returns by value, marked const. Correct.
  2. No backward compatibility concern -- this is purely additive (a new public method on a C++ class, no schema change).
  3. No security concern -- the Type enum is already public, and getChannelNumber() already reveals the type indirectly (it throws on mismatch). This just provides a non-throwing way to query it.
  4. Naming follows KJ style -- getType() is the expected getter name for a field called type.
  5. No memory safety or concurrency concern -- trivial value-type return.

There are no actionable issues with this change.

LGTM

github run

@ask-bonk ask-bonk bot requested a review from a team as a code owner March 14, 2026 02:09
"@opencode-ai/plugin": "1.2.24"
"@opencode-ai/plugin": "1.2.26"
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ugh... no thank you Bonk.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mind as well just merge it

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.

4 participants