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.

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

refactor(core): Remove deprecated TIME constant #11740

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

ivov
Copy link
Contributor

@ivov ivov commented Nov 14, 2024

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Nov 14, 2024
Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/cli/src/services/cache/cache.service.ts 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@ivov ivov marked this pull request as ready for review November 14, 2024 14:28
netroy
netroy previously approved these changes Nov 14, 2024
Copy link

cypress bot commented Nov 14, 2024

n8n    Run #7909

Run Properties:  status check passed Passed #7909  •  git commit 7e50146219: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Project n8n
Branch Review remove-deprecated-time-constant
Run status status check passed Passed #7909
Run duration 04m 31s
Commit git commit 7e50146219: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Committer Iván Ovejero
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 470
View all changes introduced in this branch ↗︎

Copy link
Contributor

✅ All Cypress E2E specs passed

@@ -160,7 +160,7 @@ export class CacheService extends TypedEmitter<CacheEvents> {
});
}

await this.cache.store.expire(key, ttlMs / Time.milliseconds.toSeconds);
await this.cache.store.expire(key, ttlMs * Time.milliseconds.toSeconds);
Copy link
Member

Choose a reason for hiding this comment

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

I also did not catch this 🤦🏽

Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Copy link
Contributor

✅ All Cypress E2E specs passed

@ivov ivov merged commit f4f0b51 into master Nov 15, 2024
36 checks passed
@ivov ivov deleted the remove-deprecated-time-constant branch November 15, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants