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

Remove SSRC async initializer #2478

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

erikeldridge
Copy link
Contributor

@erikeldridge erikeldridge commented Feb 27, 2024

Removes a method intended as a convenience, which turned out to be inconvenient.

PR #2458 defined an async convenience method getServerTemplate. In practice, we find the sync alternative initServerTemplate much more user-friendly and use it exclusively, so this PR removes getServerTemplate.

Discussion

Working with @lahirumaramba and @trekforever.

Testing

Ran npm test and all tests pass.

Functionally tested using a local server.

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

LGTM! thanks!

await template.load();
return template;
}

Copy link
Member

Choose a reason for hiding this comment

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

do we need to update the internal API proposals to reflect this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking this is a subset of the approved API, ie I'm not adding an unreviewed method, so it wasn't worth a full amendment, but I'll add a note to the doc, comment-in the approvers and see if they have any objections.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Amendment approved!

@erikeldridge erikeldridge merged commit 711dd17 into ssrc-rename-default Mar 7, 2024
5 checks passed
@erikeldridge erikeldridge deleted the ssrc-remove-async-getter branch March 7, 2024 18:06
@erikeldridge erikeldridge restored the ssrc-remove-async-getter branch March 7, 2024 18:06
erikeldridge added a commit that referenced this pull request Mar 7, 2024
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.

3 participants