It's generally better for performance to let this be async, especially if it's implemented natively and can take advantage of threading / parallelism.
Putting this as a separate diff because I think it has some separate risks.
Depends on D9294
Details
Details
See parent diff's test plan
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| keyserver/src/socket/socket.js | ||
|---|---|---|
| 263 ↗ | (On Diff #31428) | Normally it's an anti-pattern to await in sequence like this. But in this case, we have a requirement that this array of serverResponses is delivered in order. See this code |
| lib/utils/sequential-promise-resolver.js | ||
| 27 ↗ | (On Diff #31428) | Similar to above, it's important that we await in sequence here as the messages must be delivered in order. For more context, see D355 |
Comment Actions
Glad it wasn't too much overhead to use the async versions of the brotli functions
| keyserver/src/socket/socket.js | ||
|---|---|---|
| 263 ↗ | (On Diff #31428) | Might be worth adding a comment for future readers who might think of "fixing" this in the future? |