Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[N8N-4339] Unify all Cron-specific code. Improve typing, and add tests. #3887

Merged
merged 3 commits into from
Aug 19, 2022
Merged

[N8N-4339] Unify all Cron-specific code. Improve typing, and add tests. #3887

merged 3 commits into from
Aug 19, 2022

Conversation

netroy
Copy link
Member

@netroy netroy commented Aug 16, 2022

No description provided.

@netroy netroy requested a review from ivov August 16, 2022 11:50
@netroy netroy force-pushed the cron-refactor branch 2 times, most recently from 09526ce to bf75c88 Compare August 16, 2022 11:55
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request labels Aug 16, 2022
@netroy netroy requested a review from janober August 17, 2022 09:16
ivov
ivov reviewed Aug 18, 2022
View reviewed changes
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran new tests, manually tested a cron node.

packages/core/src/ActiveWorkflows.ts Outdated Show resolved Hide resolved
packages/workflow/src/Cron.ts Show resolved Hide resolved
packages/workflow/src/Cron.ts Outdated Show resolved Hide resolved
packages/workflow/src/Cron.ts Outdated Show resolved Hide resolved
packages/workflow/src/Cron.ts Outdated Show resolved Hide resolved
packages/workflow/src/Cron.ts Outdated Show resolved Hide resolved
@netroy
Copy link
Member Author

netroy commented Aug 18, 2022

@ivov updated and rebased

@netroy netroy requested a review from ivov August 19, 2022 09:58
ivov
ivov approved these changes Aug 19, 2022
View reviewed changes
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this!

@netroy netroy merged commit dc8f8b7 into n8n-io:master Aug 19, 2022
@netroy netroy deleted the cron-refactor branch August 19, 2022 10:45
@netroy netroy mentioned this pull request Oct 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivov ivov ivov approved these changes

@janober janober Awaiting requested review from janober

Assignees
No one assigned
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request tech debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@netroy @ivov