[keyserver] Introduce `getMediaMessageServerDBContentsFromMedia(...)`
ClosedPublic
Actions

Authored by atul on Sep 20 2022, 9:38 AM.

Details

Reviewers

tomek
• abosh
marcin

Commits

rCOMMff82d0f616b4: [keyserver] Introduce `getMediaMessageServerDBContentsFromMedia(...)`

Summary

Pull out common logic from multimedia-message-spec:messageContentForServerDB(...) and native/input-state-container:sendMultimediaMessageAction(...) and introduce helper function.

This is just a straightforward refactor to cut redundancy.

Depends on D5197

Test Plan

Set breakpoints and verified that values were as expected at each step.

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D5198 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Sep 20 2022, 9:38 AM

Herald added a subscriber: ashoat. · View Herald TranscriptSep 20 2022, 9:38 AM

Harbormaster completed remote builds in B12329: Diff 16921.Sep 20 2022, 9:51 AM

atul requested review of this revision.Sep 20 2022, 9:51 AM

ashoat mentioned this in D5197: [lib] Implement `messageContentForServerDB` for `messageTypes.MULTIMEDIA`.Sep 20 2022, 11:56 AM

ashoat added inline comments.

native/input/input-state-container.react.js
331 ↗	(On Diff #16921)	This is weird... shouldn't Prettier limit to 80 chars? If you format this in two lines, will Prettier force it back to one line?

tomek accepted this revision.Sep 21 2022, 7:04 AM

This revision is now accepted and ready to land.Sep 21 2022, 7:04 AM

rebase before addressing feedback and before landing

remove type annotation for mediaMessageContents

atul added inline comments.Sep 22 2022, 6:14 AM

native/input/input-state-container.react.js
331 ↗	(On Diff #16921)	If you format this in two lines, will Prettier force it back to one line? Yeah, I just ended up removing the type annotation which brought the line width to 75. Guessing you've seen the following from the Prettier docs since this has come up a few times in the past: Prettier’s printWidth option does not work the same way. It is not the hard upper allowed line length limit. It is a way to say to Prettier roughly how long you’d like lines to be. Prettier will make both shorter and longer lines, but generally strive to meet the specified printWidth. (https://prettier.io/docs/en/options.html#print-width) Usually I've seen that result in lines that are ~90ish lines long, 120ish seems way beyond "roughly" 80. Maybe `prettier` can't break apart `$ReadOnlyArray<MediaMessageServerDBContent>`? Just a hunch I have no idea