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

Refactor hooks naming #2605

Merged
merged 8 commits into from
Jan 29, 2023
Merged

Refactor hooks naming #2605

merged 8 commits into from
Jan 29, 2023

Conversation

davidsoderberg
Copy link
Contributor

What change does this PR introduce?

Refactor naming of all hooks

Why was this change needed?

To make our hook naming consistent

@ghost
Copy link

ghost commented Jan 25, 2023 •
edited by ghost
Loading

👇 Click on the image for a new way to code review

  • Make big changes easier — review code in small groups of related files

  • Know where to start — see the whole change at a glance

  • Take a code tour — explore the change with an interactive tour

  • Make comments and review — all fully sync’ed with github

    Try it now!

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map Legend

LetItRock
LetItRock approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@LetItRock LetItRock left a comment •
edited
Loading

Choose a reason for hiding this comment

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

please check the failing pipeline DeepSource and resolve conflicts ;)

p-fernandez
p-fernandez approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@p-fernandez p-fernandez left a comment

Choose a reason for hiding this comment

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

🌟

Open question. Should we also update the notification center package with the same convention?

@LetItRock
Copy link
Contributor

LetItRock commented Jan 25, 2023 •
edited
Loading

@p-fernandez @davidsoderberg I've updated all the hooks in the notification-center in this PR: #2382

scopsy
scopsy approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@scopsy scopsy left a comment

Choose a reason for hiding this comment

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

🎉

@davidsoderberg davidsoderberg merged commit 7542b1d into next Jan 29, 2023
@davidsoderberg davidsoderberg deleted the refactor-hooks-naming branch January 29, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LetItRock LetItRock LetItRock approved these changes

@scopsy scopsy scopsy approved these changes

@p-fernandez p-fernandez p-fernandez approved these changes

@ainouzgali ainouzgali Awaiting requested review from ainouzgali

@BiswaViraj BiswaViraj Awaiting requested review from BiswaViraj

@djabarovgeorge djabarovgeorge Awaiting requested review from djabarovgeorge

Assignees
No one assigned
Labels
@novu/web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@davidsoderberg @LetItRock @scopsy @p-fernandez