<greg> Summary of Chat:
- Pierre: info icon disappears on scroll (#1234) + 6 months support history mobile version + PR feedback
- Leihla: Mobile version of support history widget + Document new proposals widget + Dark theme system settings (#985)
- Ricardo: Implementing key management for joining a group
- Greg: Review Ricardo’s PR + Review Alex’s PR + Review Pierre’s PR + Get started implementing contract versions (#1214) (Maybe: review other PRs)
- Andrea: Schedule A to 100% + working with Mae on blogs & grants
- Alex: TBD
- Snowteamer: TBD
Preliminary Success Rate: 90% ! (Pending Alex)
<Snowteamer> Regarding that last issue, not sure whether converting appLogsFilter from an array to a plain object is a valid optimization as it only has up to four elements.
If app log capturing performance should be questioned or improved, I think I'd first look for a way to reduce the frequency of local storage calls. I'm not sure but they are likely slow.
Other smaller changes might also be considered, like replacing the Proxy object by plain hooks. This might remove one level of indirection when calling console methods.
<greg> @Snowteamer
Regarding that last issue, not sure whether converting
appLogsFilterfrom an array to a plain object is a valid optimization as it only has up to four elements.
Fair point.If app log capturing performance should be questioned or improved, I think I’d first look for a way to reduce the frequency of local storage calls. I’m not sure but they are likely slow.
True… although, AFAIK they are done asynchronously right? And we don’t await on them.Other smaller changes might also be considered, like replacing the
Proxyobject by plain hooks. This might remove one level of indirection when calling console methods.
Interesting idea 👍
cold out.
'backend/db/streamEntriesBefore' here: https://github.com/okTurtles/group-income/pull/1274/files#r878461748I don’t know though how to properly handle letting the caller know of an error during the stream though, and the same issue exists in
'backend/db/streamEntriesSince'. Maybe either you or @Snowteamer have ideas for that
(Btw, I will assign the Deno issue to you, so that others know you’re working on it)
ReadableStream, that has not much to do with the original question but I would add that Deno has a quite different API (Readable) , so using these APIs directly is not that great for compatibility.Also, I understand that we're streaming entries from the server for improved throughput, but on the client-side it looks like we're waiting for all entries to be received before going on. Does streaming still makes sense in that case? Maybe it really does. I'm just not sure here.
<greg> @Snowteamer thanks for the replies! Hopefully it help @Alex Jin.
Does streaming still makes sense in that case? Maybe it really does. I’m just not sure here.
Well, that’s a fair point, it just means that if needed, the client-side of the code could be improved to take advantage of it
<greg> ✅ Review Alex’s PR: https://github.com/okTurtles/group-income/pull/1274#pullrequestreview-980739154
Great job @Alex Jin! 😄
<greg> ✅ Review @Pierre’s PR: https://github.com/okTurtles/group-income/pull/1278#pullrequestreview-980750053
Also fantastic work! 🙂
<greg> ✅ Review @Ricardo Iván Vieitez Parra’s PR. https://github.com/okTurtles/group-income/pull/1257#pullrequestreview-980771674
Fantastic progress! This is a preliminary review and I will wait for replies to the various comments (old unaddressed ones & new) before proceeding with a more in-depth review.