Page MenuHomePhabricator

Implement draft store operations processing in CommCoreModule
ClosedPublic

Authored by marcin on Nov 16 2022, 5:16 AM.
Tags
None
Referenced Files
F3360621: D5648.diff
Sun, Nov 24, 1:39 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:22 PM
Unknown Object (File)
Tue, Nov 19, 2:21 PM
Subscribers

Details

Summary

This differential implements draft store processing functionality in CommCoreModule to enable bulk drafts dump to SQLite.

Test Plan

Make sure eslint and flow do not point errors.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Looks good!

Easy to review as it closely follows the conventions from processThreadStoreOperations(...) and processMessageStoreOperations(...)

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
357 ↗(On Diff #18473)

Personally prefer being explicit in the lambda capture list, but don't think we ever determined any "team convention" so totally up to you

373 ↗(On Diff #18473)

Same thing where I'd personally prefer being explicit about the lambda capture list, but up to you

This revision is now accepted and ready to land.Nov 17 2022, 3:22 PM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
373 ↗(On Diff #18473)

Here and above I was following the code for processMessageStoreOperations and processThreadStoreOperations where lambda captures are also as simple as this.

tomek requested changes to this revision.Nov 18 2022, 4:01 AM
tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
317–318 ↗(On Diff #18473)

It's a really bad practice to use #define constants (in ancient times this was the solution, but now it's ok to use const). The most important drawback of using #define is that the name will disappear and during the runtime (e.g. debugging) there's only the value. So you won't see MOVE_DRAFT_OPERATION when debugging, it will be just "move" and that will make this process a lot harder.

This revision now requires changes to proceed.Nov 18 2022, 4:01 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
317–318 ↗(On Diff #18473)

Sure. I will also replace #define with constants for MessageStore and ThreadStore operations.

Use const instead of define

Process remove all drafts operation in CommCoreModule

This revision is now accepted and ready to land.Nov 21 2022, 4:40 AM