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

CRDT sync handles semantic domains #1217

Merged
merged 9 commits into from
Nov 11, 2024
Merged

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Nov 6, 2024

Fix #1216.

Currently this PR is based on top of #1203 for convenience so I can more easily bootstrap the semantic domain implementation using the parts of speech sync. Once #1203 is merged, this PR should auto-base onto develop instead.

We now sync semantic domains just like we sync parts of speech. There are similar issues with deciding how Predefined should be set; I'm currently setting it to true for any semantic domain coming from FW, because custom semantic domains will be rare. But ultimately we'll want to keep a list of GUIDs around and match the incoming semdom against that list of GUIDs; if it's not on the list then Predefined should be false instead.

@rmunn rmunn self-assigned this Nov 6, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Nov 7, 2024

Tests are now working. I'm going to rebase on top of develop after merging #1203, then this is going to be ready for review.

Base automatically changed from feat/crdt-sync-parts-of-speech to develop November 7, 2024 09:54
@rmunn rmunn force-pushed the feat/crdt-sync-semantic-domains branch from 08c286c to 8716ebe Compare November 7, 2024 09:57
Copy link

github-actions bot commented Nov 7, 2024

C# Unit Tests

75 tests   75 ✅  5s ⏱️
13 suites   0 💤
 1 files     0 ❌

Results for commit 0885ca6.

♻️ This comment has been updated with latest results.

@rmunn rmunn marked this pull request as ready for review November 7, 2024 10:02
@rmunn rmunn requested a review from hahn-kev November 7, 2024 10:02
- Add GetSemanticDomain() to read API
- CreateSemanticDomain now returns the created object
- Add UpdateSemanticDomain() to write API
- Add DeleteSemanticDomain() to write API
- Add UpdateSemanticDomainProxy class
- Add SemanticDomainSync class
- Sync semantic domains in CRDT-FW sync service
@rmunn rmunn force-pushed the feat/crdt-sync-semantic-domains branch from 8716ebe to b16957c Compare November 11, 2024 02:49
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks good, I changed <EnableDefaultContentItems>false</EnableDefaultContentItems> because that will also exclude appsettings.json

@rmunn rmunn merged commit 85f9555 into develop Nov 11, 2024
16 checks passed
@rmunn rmunn deleted the feat/crdt-sync-semantic-domains branch November 11, 2024 09:07
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.

Sync semantic domains in CrdtMerge
2 participants