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(app-gen): cache locking #5586

Draft
wants to merge 2 commits into
base: next
Choose a base branch
from
Draft

Conversation

djabarovgeorge
Copy link
Contributor

@djabarovgeorge djabarovgeorge commented May 16, 2024

What changed? Why was the change needed?

This pull request is a work in progress and shouldn't be reviewed yet. It's the first step in a refactoring process, and the next stage will focus on reusing the applyLock method.

I'm also questioning where the locking logic belongs. While I like the cleanness of this solution, I'm unsure if the cache layer is the ideal place for it. Perhaps it should reside within a separate use case. I'd appreciate your thoughts on this matter.

Screenshots

Expand for optional sections

Related enterprise PR

EE

Special notes for your reviewer

Copy link

netlify bot commented May 16, 2024

Deploy Preview for dev-web-novu failed. Why did it fail? →

Name Link
🔨 Latest commit dfadfe9
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/665477d3e1e37b000826c126

Copy link

netlify bot commented May 16, 2024

Deploy Preview for novu-design failed. Why did it fail? →

Name Link
🔨 Latest commit dfadfe9
🔍 Latest deploy log https://app.netlify.com/sites/novu-design/deploys/665477d3344fb300081e3d31

Copy link

Hey there and thank you for opening this pull request! 👋

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Your PR title is: refactor(app-gen): cache locking
It should be something like: feat(scope): Add fancy new feature

Details:

Unknown scope "app-gen" found in pull request title "refactor(app-gen): cache locking". Scope must match one of: root, api, inbound-mail, web, webhook, widget, worker, ws, ee-auth, ee-billing, ee-billing-web, ee-echo-api, ee-echo-web, ee-echo-worker, ee-dal, ee-shared-services, ee-translation, ee-translation-web, application-generic, automation, dal, design-system, embed, novui, shared, shared-web, testing, novu, novu-labs, client, echo, headless, nest, node, notification-center, angular-workspace, notification-center-angular, notification-center-vue, providers, stateless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant