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

Don't directly export cap table from message #498

Merged
merged 7 commits into from
Apr 3, 2023

Conversation

lthibault
Copy link
Collaborator

This PR paves the way for adding refcounting to capnp.Message. Prior attempts to do so where frustrated by other packages accessing Message.CapTable directly. This PR defines a Message.CapTable() method, that returns a new CapTable type. The cap table is accessed and updated through the latter's methods. This has the side benefit of simplifying code in quite a few places.

Note that this PR ended up rather large, but that most of the changes involve updating calls to Message.AddCap etc. The diff is actually quite simple beyond that. I recommend reviewing each PR separately, and focusing your attention on message.go and captable.go.

@lthibault lthibault requested a review from zenhack April 2, 2023 01:16
@lthibault lthibault force-pushed the prep/dont-export-msg-captable branch 4 times, most recently from afbd61d to e2efc56 Compare April 2, 2023 01:41
@lthibault lthibault force-pushed the prep/dont-export-msg-captable branch from 91a389a to da81dcf Compare April 2, 2023 23:54
Copy link
Contributor

@zenhack zenhack left a comment

Choose a reason for hiding this comment

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

Mostly looks good, a couple comments in-line, one trivial and one critical.

message.go Show resolved Hide resolved
rpc/rpc.go Outdated Show resolved Hide resolved
@lthibault lthibault requested a review from zenhack April 3, 2023 17:01
@lthibault lthibault self-assigned this Apr 3, 2023
@zenhack zenhack merged commit 70259c7 into main Apr 3, 2023
@zenhack zenhack deleted the prep/dont-export-msg-captable branch April 3, 2023 19:43
@zenhack zenhack restored the prep/dont-export-msg-captable branch April 3, 2023 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants