Skip to content

Add 'sbp/domains/lock'#4

Merged
taoeffect merged 3 commits intookTurtles:masterfrom
snowteamer:implement-lock
Jul 7, 2023
Merged

Add 'sbp/domains/lock'#4
taoeffect merged 3 commits intookTurtles:masterfrom
snowteamer:implement-lock

Conversation

@snowteamer
Copy link
Copy Markdown
Contributor

Closes #2

Copy link
Copy Markdown
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

While the implementation of 'sbp/domains/lock' seems correct, this PR doesn't actually do any locking, but only gives the appearance of locking domains.

@snowteamer
Copy link
Copy Markdown
Contributor Author

My bad, you're right. It seems like I forgot to update 'sbp/selectors/register' accordingly.

Copy link
Copy Markdown
Member

@taoeffect taoeffect left a comment

Choose a reason for hiding this comment

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

Fantastic work @snowteamer!! Thanks for updating the tests too! 😄

SBP is now one more step closer to perfection! 😁

}
} else for (const name of typeof domainNameOrNames === 'string' ? [domainNameOrNames] : domainNameOrNames) {
if (!domains[name]) {
throw new Error(`SBP: unknown or invalid domain name: ${name}`)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should probably simplify this error message to SBP: cannot lock non-existent domain: ${name}

@taoeffect taoeffect merged commit e51dadc into okTurtles:master Jul 7, 2023
for (const domain of Object.values(domains)) {
domain.locked = true
}
} else for (const name of typeof domainNameOrNames === 'string' ? [domainNameOrNames] : domainNameOrNames) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, this else should have a { after it, but I'll fix that in a followup.

for (const selector in sels) {
const domain = domainFromSelector(selector)
if (selectors[selector]) {
const domainName = domainFromSelector(selector)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This actually breaks line 59 above:

if (selector === `${domain}/_init`) {

We both forgot to run npm run flow which caught this error.

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.

Add a 'sbp/domains/lock' selector

2 participants