New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add error message if unable to find cron expression for scheduled function #1123
Conversation
β± Benchmark resultsComparing with 7e21b63 largeDepsEsbuild: 7.7sβ¬οΈ 26.18% increase vs. 7e21b63
LegendlargeDepsNft: 39.3sβ¬οΈ 23.63% increase vs. 7e21b63
LegendlargeDepsZisi: 55.1sβ¬οΈ 20.61% increase vs. 7e21b63
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the simplicity of the approach. I thought there would be a lot more changes involved. Besides the one case that is missing (see comments), I think this is the way to go.
...tures/in-source-config/functions_missing_cron_expression/cron_cjs_invalid_cron_expression.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello from pto :) lgtm
If you want a release after merging you would need to change the type in the squash commit or pr title if using kodiak from chore to fix
π Thanks for submitting a pull request! π
Summary
Fixes #<1108>
Checks to make sure that scheduled functions aren't missed. If the
schedule
helper is imported but not correctly invoked/exported, assumes user wanted to schedule the function and throws an error.For us to review and ship your PR efficiently, please perform the following steps:
This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing
a typo or something that`s on fire π₯ (e.g. incident related), you can skip this step.
passes our tests.
A picture of a cute animal (not mandatory, but encouraged)