-
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
integrate icon plugin into MediaServer #369
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.
Looks good to me. One small non-blocking change: There is a TODO about a failing test in tests/icon-api.js, but I think that test works now.
Not sure how this PR would fix it, as I directly address in #370 (which re-enables it) |
also removes the projectPublicId constructor opt from both IconApi and BlobApi. unnecessary given that it's only used by the getMediaBaseUrl opt, so makes more sense to just handle it there.
a8f755a
to
f132e4a
Compare
Sorry, I got the blobs api confused with the icon api Test still fails though |
* fix/local-peers: new tests for duplicate connections fixed fake timers implementation for tests send invite to non-existent peer fix stream close before channel open fix: handle duplicate connections to LocalPeers fix: fix writing to blob store when also calculating content hash (#370) integrate icon plugin into MediaServer (#369) chore: Add debug logging (#373)
Closes #329
Changes grew a little in scope to account for some refactoring opportunities that I saw, namely:
projectPublicId
constructor opt fromBlobApi
andIconApi
in favor of using the id when defining thegetMediaBaseUrl()
opt