Note to reviewers: this is addressing an Urgent issue, so would be great to prioritize!
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
Feed All Stories
All Stories
All Stories
Mar 21 2023
Mar 21 2023
ashoat added a comment to D7106: Native Java code to request notifications permissions from JS on Android 13.
(My comments are all repeats of my previous review)
I can change it either way, but we should decide whether we possibly want to use this table for calendar entires in the future. The benefit of using the same table for messages and entries is that if we fetch them by for example time, we will get them sorted correctly right away, and won't have to think about how many of each we should fetch
Can you clarify in what scenarios the id can be null? Usually we only see that for types that also have localID
Naming question
marcin updated the diff for D7106: Native Java code to request notifications permissions from JS on Android 13.
Mirror iOS behavior on Android 13
The test plan is D7114, but that diff failed the Android build. Can you fix it up so that the Android build is no longer breaking? I'm also curious how you were able to test the Android build... was it succeeding in your environment despite failing in CI?
pageSize is 1001 because I used the idea from https://mariadb.com/kb/en/pagination-optimization/ that is to fetch one more row than we intend to process.
rename function to be more detailed
inka updated the summary of D7117: [keyserver] Add migration - process messages in our db for search.
Rebase
Address review, rebase
marcin planned changes to D7106: Native Java code to request notifications permissions from JS on Android 13.
I will change JS-visible interface of those methods.
Removed localID from the type
tomek added inline comments to D6929: [keyserver] Add a function to toggle the pin status of a message.
@inka fixed this in another diff.
Mar 20 2023
Mar 20 2023
Fix what information is being requested by whom
[Comm-opaque2] Add wasm build to client
rohan added inline comments to D7110: [keyserver/lib] Add a fetchPinnedMessages method to retrieve all pinned messages for a thread.
rohan updated the diff for D7110: [keyserver/lib] Add a fetchPinnedMessages method to retrieve all pinned messages for a thread.
Rename types
Wonder if there's a way to catch this sort of thing with CI
[lib] Fix `TUnion`/`TInterface` imports
atul committed rCOMMbcb8e9e4126e: [lib] Introduce `ClientAvatar` and introduce `avatar` field for `*UserInfo`… (authored by atul).
[lib] Introduce `ClientAvatar` and introduce `avatar` field for `*UserInfo`…
atul updated the diff for D7109: [lib] Introduce `ClientAvatar` and introduce `avatar` field for `*UserInfo` types.
rebase and land
Harbormaster failed remote builds in B17445: Diff 23900 for D7099: [native] add feature flag to avatars render work!
Harbormaster failed remote builds in B17444: Diff 23899 for D7069: [native] render the rest of the user avatars!
Harbormaster failed remote builds in B17443: Diff 23898 for D7057: [native] render avatar in profile screen!
atul committed rCOMMe927714acf3f: [keyserver] Introduce `updateUserAvatarResponder` endpoint scaffolding (authored by atul).
[keyserver] Introduce `updateUserAvatarResponder` endpoint scaffolding
rebase before landing
rebase before landing
rebase before landing
rebase before landing
atul updated the diff for D7108: [keyserver] Introduce `updateUserAvatarResponder` endpoint scaffolding.
rebase and land
feedback
rebase before landing
ginsu updated the diff for D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
implemented atuls quick suggestion
atul committed rCOMM4c6ac988e277: [lib] Introduce `updateUserAvatarRequestValidator` (authored by atul).
[lib] Introduce `updateUserAvatarRequestValidator`
rebase and land
ginsu added inline comments to D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
I'm not sure, but I think we should bump buildToolsVersion as well
ginsu updated the diff for D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
fix gates
In D7060#210983, @ashoat wrote:{F436096}
Putting it in scripts/ sounds like a good idea to me!
atul requested changes to D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
One last change
ginsu added inline comments to D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
ginsu updated the diff for D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
added avatar client feature gate to fetcher
This diff seems to call getAPNsNotificationTopic with two parameters, but the version of that function in master only takes one parameter. Can you please update this diff's dependencies so I can see the rest of your stack? Please always set diff dependencies!
atul added inline comments to D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
Almost there!
atul added 1 blocking reviewer(s) for D7109: [lib] Introduce `ClientAvatar` and introduce `avatar` field for `*UserInfo` types: ginsu.
atul published D7109: [lib] Introduce `ClientAvatar` and introduce `avatar` field for `*UserInfo` types for review.
ginsu updated the diff for D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
improvements
Still need to fix chat bubble poking out (will timebox for 1-2hrs)
Looks good, but a couple notes in message-creator.js
looks good thanks for making the unit tests
ginsu added inline comments to D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
Can you create a Linear task for handling this edge case and set it to Backlog?
Putting it in scripts/ sounds like a good idea to me!
ginsu updated the diff for D7054: [keyserver/lib] fetch avatar column from users table in fetchCurrentUserInfo.
address comments
atul updated the diff for D7108: [keyserver] Introduce `updateUserAvatarResponder` endpoint scaffolding.
fix typo