[keyserver] Extract getBackupInfos
ClosedPublic
Actions

Authored by ashoat on Jun 14 2022, 10:28 AM.

Details

Reviewers

atul
• abosh

Commits

rCOMM1bc6faac28eb: [keyserver] Extract getBackupInfos

Summary

We're going to want to reuse some of this code for ENG-1234, so I extracted it.

Depends on D4262

Test Plan

I copy pasted the new backups.js file into prod and added a console.log of the result of getBackupInfos. I made sure it was in chrono order and had everything right

Diff Detail

Repository

rCOMM Comm

Branch

ashoat/backups

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Jun 14 2022, 10:28 AM

Herald added subscribers: • adrian, tomek. · View Herald TranscriptJun 14 2022, 10:28 AM

Harbormaster completed remote builds in B9749: Diff 13497.Jun 14 2022, 10:37 AM

ashoat requested review of this revision.Jun 14 2022, 10:37 AM

atul accepted this revision.Jun 14 2022, 11:55 AM

atul added inline comments.

keyserver/src/cron/backups.js
223 ↗	(On Diff #13497)	Thoughts on naming this something like `getSortedBackupInfos` or something? Just something to indicate at the callsite that we're getting the data in some order so the `[0]` seems less arbitrary?
223 ↗	(On Diff #13497)	Can we make this return type `$ReadOnlyArray<BackupInfo`?
230 ↗	(On Diff #13497)	We could maybe sequence the `startsWith` before the `endsWith`... but really doesn't matter.

This revision is now accepted and ready to land.Jun 14 2022, 11:55 AM

ashoat added inline comments.Jun 15 2022, 9:59 AM

keyserver/src/cron/backups.js
223 ↗	(On Diff #13497)	`getSortedBackupInfos` sounds good. I'd rather leave the array as mutable. Since we're returning a new array, it should be up to the caller if they want to mutate
230 ↗	(On Diff #13497)	Fair