-
Notifications
You must be signed in to change notification settings - Fork 44
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
Feat/9 nv go 2 api 2 get all notifications #18
base: master
Are you sure you want to change the base?
Feat/9 nv go 2 api 2 get all notifications #18
Conversation
@prajjwaldimri can you help review this? |
Please can you update the readme? @samsoft00 |
Sure, I'll also add some tests. |
@samsoft00 If possible, please convert the query params in this PR to use structs and the new utils function ( Line 24 in 794b4dd
|
Nope, I have reservations regarding this approach, aside, I need to handle array and array string in my current implementation, I'll communicate before Wednesday.
|
Oh, okay. Let's discuss your reservations and land on one approach for the library, then. Otherwise, different endpoints would have different ways of passing data to them, which would result in a bad experience.
|
@prajjwaldimri @samsoft00 Any updates here? |
Hey guys @prajjwaldimri @samsoft00 It's been 3 weeks since this PR came up. Can we make a decision asap? If no decision is made by next week, I'll have to go with an option so we can close this out. 🙏🏿 |
@samsoft00 @prajjwaldimri please any updates here? |
@unicodeveloper This is blocked on @samsoft00. |
@samsoft00 Please can you update the readme? I'll like to merge this in as soon as possible. There are folks requesting for this feature 🙏🏿 |
@samsoft00 @prajjwaldimri any updates on this? |
@samsoft00 there are conflicts that we need to resolve |
+1 |
Issue: #9