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

feat(js): Com 208 improve the dx of the novu on function to return the cleanup #6650

Merged
merged 3 commits into from
Oct 15, 2024
Merged

feat(js): Com 208 improve the dx of the novu on function to return the cleanup #6650

merged 3 commits into from
Oct 15, 2024

Conversation

BiswaViraj
Copy link
Member

What changed? Why was the change needed?

  • return cleanup function instead of separate off method

Screenshots

Expand for optional sections

Related enterprise PR

Special notes for your reviewer

@linear Linear
Copy link

linear bot commented Oct 8, 2024

COM-208 Improve the DX of the `novu.on` function to return the cleanup function

@netlify Netlify
Copy link

netlify bot commented Oct 8, 2024 •
edited
Loading

✅ Deploy Preview for novu-stg-vite-dashboard-poc ready!

Name Link
🔨 Latest commit 7c14288
🔍 Latest deploy log https://app.netlify.com/sites/novu-stg-vite-dashboard-poc/deploys/670cd0bae57a2e00081264f7
😎 Deploy Preview https://deploy-preview-6650--novu-stg-vite-dashboard-poc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

SokratisVidros
SokratisVidros approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@SokratisVidros SokratisVidros left a comment

Choose a reason for hiding this comment

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

@BiswaViraj Reminder to also update the docs

desiprisg
desiprisg previously requested changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@desiprisg desiprisg left a comment

Choose a reason for hiding this comment

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

We need to keep the existing functionality (off) as well, since removing it is a breaking change.

@SokratisVidros SokratisVidros changed the title feat(js): Com 208 improve the dx of the novuon function to return the cleanup feat(js): Com 208 improve the dx of the novu on function to return the cleanup Oct 9, 2024
@BiswaViraj BiswaViraj force-pushed the com-208-improve-the-dx-of-the-novuon-function-to-return-the-cleanup branch from 8fea9f3 to df4ca5a Compare October 14, 2024 07:32
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Oct 14, 2024 •
edited
Loading

Open in Stackblitz

pnpm add https://pkg.pr.new/novuhq/novu/@novu/js@6650

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react@6650

pnpm add https://pkg.pr.new/novuhq/novu/@novu/react-native@6650

commit: 7c14288

@BiswaViraj BiswaViraj requested a review from desiprisg October 14, 2024 08:05
@BiswaViraj BiswaViraj dismissed desiprisg’s stale review October 15, 2024 11:14

applied the suggestions

@BiswaViraj BiswaViraj merged commit 394bacb into next Oct 15, 2024
37 checks passed
@BiswaViraj BiswaViraj deleted the com-208-improve-the-dx-of-the-novuon-function-to-return-the-cleanup branch October 15, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SokratisVidros SokratisVidros SokratisVidros approved these changes

@LetItRock LetItRock Awaiting requested review from LetItRock

@desiprisg desiprisg Awaiting requested review from desiprisg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BiswaViraj @SokratisVidros @desiprisg