[keyserver] Bump `node-schedule` to `2.1.0`
ClosedPublic
Actions

Authored by atul on Apr 27 2022, 7:57 AM.

Details

Reviewers

• jacek
tomek
ashoat

Commits

rCOMM1eb5c8ec7cb6: [keyserver] Bump `node-schedule` to `2.1.0`

Summary

Downstream dependencies have issues surfaced by yarn audit

Checked the release notes: https://github.com/node-schedule/node-schedule/releases and there don't seem to be any breaking changes that affect us:

It looks like the two breaking changes since are

Drop support for Node < 6

which doesn't affect us

"This removes support for passing job objects with an execute() method instead of a function... This is technically a breaking change, although this "feature" was never documented."

which doesn't affect us either. checked cron.js which is the only place we use node-schedule and we aren't using the execute() approach anywhere

Depends on D3857

Test Plan

Diff Detail

Repository

rCOMM Comm

Branch

landapr27 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 27 2022, 7:57 AM

Herald added subscribers: • abosh, • karol, • adrian. · View Herald TranscriptApr 27 2022, 7:57 AM

atul edited the summary of this revision. (Show Details)Apr 27 2022, 7:59 AM

Harbormaster completed remote builds in B8604: Diff 11988.Apr 27 2022, 8:02 AM

atul requested review of this revision.Apr 27 2022, 8:02 AM

atul edited the summary of this revision. (Show Details)Apr 27 2022, 8:17 AM

atul added a parent revision: D3857: [yarn] Bump transitive dependencies of `expo`.

atul added a child revision: D3859: [yarn] Bump `clang-format` to `1.7.0`.

Please make sure to make me a blocking reviewer on diffs that affect dependencies in the future
Can you also test wherever we use node-schedule to make sure it still works as expected? You can adjust the schedule of some cronjob to make it run every minute or something

This revision is now accepted and ready to land.Apr 27 2022, 2:31 PM

Please make sure to make me a blocking reviewer on diffs that affect dependencies in the future

Was planning on adding you as blocking reviewer before landing if @palys-swm had accepted first. Can add you as blocking reviewer immediately in the future.

No downside to adding me as a blocking reviewer right away, I think

In D3858#107712, @ashoat wrote:

No downside to adding me as a blocking reviewer right away, I think

Makes sense. I thought one of the goals was to reduce the size of your "Must Review" queue, but I must have been mistaken. Did add you as first-pass reviewer though so it'd show up in "Ready to Review."

I thought one of the goals was to reduce the size of your "Must Review" queue, but I must have been mistaken

Nah that's not really a goal I think... but I see where you're coming from now

atul mentioned this in D3857: [yarn] Bump transitive dependencies of `expo`.Apr 28 2022, 11:30 AM

rebase before landing

Can you also test wherever we use node-schedule to make sure it still works as expected? You can adjust the schedule of some cronjob to make it run every minute or something

We exclusively use node-schedule in cron.js. I tested this by logging "CRON" every minute and verifying that it appeared in the keyserver output. I think this is a reasonable indication that node-schedule is still working as expected? Open to any other suggestions if it's not.