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

Added basic operations on notification templates #30

Merged
merged 2 commits into from
May 25, 2023

Conversation

Badabum
Copy link

@Badabum Badabum commented May 18, 2023

Implemented:

  • List all templates
  • Get template by id
  • Delete template
  • Update template status

Implemented:
- List all templates
- Get template by id
- Delete template
- Update template status
@Badabum Badabum mentioned this pull request May 18, 2023
5 tasks
@Badabum
Copy link
Author

Badabum commented May 19, 2023

@unicodeveloper I fixed your remarks, hope it's more readable now

@unicodeveloper
Copy link
Contributor

@unicodeveloper I fixed your remarks, hope it's more readable now

Yes, this is good!

@Badabum
Copy link
Author

Badabum commented May 20, 2023

Think we can merge it if no other comments. @wh1337 said before that failing tests are ok for now, bc it's a ci problem

@wh1337
Copy link
Collaborator

wh1337 commented May 20, 2023

Can you attach a screenshot of passing tests locally whilst we fix the CI issue?

@Badabum
Copy link
Author

Badabum commented May 20, 2023

Can you attach a screenshot of passing tests locally whilst we fix the CI issue?

image

@unicodeveloper
Copy link
Contributor

LGTM ⭐

@unicodeveloper
Copy link
Contributor

Please update the readme @Badabum

@Badabum
Copy link
Author

Badabum commented May 22, 2023

Please update the readme @Badabum

Sure, will do

@wh1337
Copy link
Collaborator

wh1337 commented May 25, 2023

Just got back into the office and reviewing this will be reviewed and merged tonight @Badabum

@wh1337 wh1337 self-requested a review May 25, 2023 13:43
@wh1337
Copy link
Collaborator

wh1337 commented May 25, 2023

Review the comment I put on the review, once resolved we can rebase it on to main.

@unicodeveloper
Copy link
Contributor

Thanks @wh1337 please this has to go in today. 🙏🏿

@wh1337 wh1337 merged commit 228969f into novuhq:main May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants