Skip to content

Use trait instead of channel for API between node and doc crate#67

Merged
jsparber merged 8 commits into
mainfrom
jsparber/use_trait
Apr 25, 2025
Merged

Use trait instead of channel for API between node and doc crate#67
jsparber merged 8 commits into
mainfrom
jsparber/use_trait

Conversation

@jsparber
Copy link
Copy Markdown
Collaborator

No description provided.

@jsparber jsparber force-pushed the jsparber/use_trait branch 4 times, most recently from 5a21c87 to af005f3 Compare March 19, 2025 18:04
@jsparber jsparber force-pushed the jsparber/use_trait branch from d83cbe6 to 5b76688 Compare March 21, 2025 12:32
@jsparber jsparber marked this pull request as ready for review March 21, 2025 13:27
@jsparber jsparber mentioned this pull request Mar 24, 2025
@jsparber jsparber force-pushed the jsparber/use_trait branch from 5b76688 to d3785bd Compare April 2, 2025 09:38
@adzialocha adzialocha self-requested a review April 24, 2025 14:35
Copy link
Copy Markdown
Member

@adzialocha adzialocha left a comment

Choose a reason for hiding this comment

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

Looking all good! Thank you for this refactoring, you successfully reduce the channel surface as much as possible 😎

I only have minor comments.

Comment thread .gitignore
Comment thread aardvark-doc/src/document.rs Outdated
Comment thread aardvark-doc/src/document.rs
Comment thread aardvark-doc/src/lib.rs
This also renames the `aardvark-node::Document` to
`aardvark-node::DocumentId`, since we now have the
`SubscribableDocument` trait it should return a `DocumentId` not a `Document`
that is actually only an id.
@jsparber jsparber force-pushed the jsparber/use_trait branch from d3785bd to daecc46 Compare April 25, 2025 14:27
Creating and subscribing to a loro doc is quite straight forward. I
think having an abstraction between the `Document` and loro makes the
code unnecessarily more complicated. Also if we decide to run the CRDT
in a distinct thread we can use the `Document` object directly since
it's Send+Sync.
@jsparber jsparber force-pushed the jsparber/use_trait branch from daecc46 to 23b2391 Compare April 25, 2025 14:32
@jsparber jsparber merged commit f4783c1 into main Apr 25, 2025
@adzialocha adzialocha deleted the jsparber/use_trait branch May 26, 2025 09:46
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