-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: define sync capability per namespace #319
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have a question but i'd consider it non-blocking
return DEFAULT_CAPABILITIES[BLOCKED_ROLE_ID] | ||
// When no role assignment exists, e.g. a newly added device which has | ||
// not yet synced role records. | ||
return NO_ROLE_CAPABILITIES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this just a few lines above:
const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth')
do you have to wrap that in a try/catch, and return this no role capabilities in the case of the catch? with #317, that call may throw an error if the document isn't found.
Basically, should this block look like this?
try {
const authCoreId = await this.#coreOwnership.getCoreId(deviceId, 'auth')
if (authCoreId === this.#projectCreatorAuthCoreId) {
return CREATOR_CAPABILITIES
} else {
// When no role assignment exists, e.g. a newly added device which has
// not yet synced role records.
return NO_ROLE_CAPABILITIES
}
} catch (err) {
// When no role assignment exists, e.g. a newly added device which has
// not yet synced role records.
return NO_ROLE_CAPABILITIES
}
* main: chore: update github action steps (#322) fix: fix mdns discovery (finally) (#323) chore: check npm lockfile version does not change (#321) chore: commit hooks for lint & package-lock chk (#320) feat: define sync capability per namespace (#319) feat: add namespace to peer pre-have messages (#314)
Fixes #318