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

Feat/9 nv go 2 api 2 get all notifications #18

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

samsoft00
Copy link
Collaborator

@samsoft00 samsoft00 commented Apr 3, 2023

Issue: #9

@samsoft00
Copy link
Collaborator Author

@prajjwaldimri can you help review this?

@samsoft00 samsoft00 self-assigned this Apr 4, 2023
@unicodeveloper
Copy link
Contributor

Please can you update the readme? @samsoft00

@samsoft00
Copy link
Collaborator Author

Sure, I'll also add some tests.

@prajjwaldimri
Copy link
Contributor

@samsoft00 If possible, please convert the query params in this PR to use structs and the new utils function (

func GenerateQueryParamsFromStruct[T interface{}](queryParamsStruct T) ([]QueryParam, error) {
), which can dynamically generate params list.

@samsoft00
Copy link
Collaborator Author

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.

@samsoft00 If possible, please convert the query params in this PR to use structs and the new utils function (

func GenerateQueryParamsFromStruct[T interface{}](queryParamsStruct T) ([]QueryParam, error) {

), which can dynamically generate params list.

@prajjwaldimri
Copy link
Contributor

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.

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.

@samsoft00 If possible, please convert the query params in this PR to use structs and the new utils function (

func GenerateQueryParamsFromStruct[T interface{}](queryParamsStruct T) ([]QueryParam, error) {

), which can dynamically generate params list.

@unicodeveloper
Copy link
Contributor

@prajjwaldimri @samsoft00 Any updates here?

@unicodeveloper
Copy link
Contributor

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. 🙏🏿

@unicodeveloper
Copy link
Contributor

@samsoft00 @prajjwaldimri please any updates here?

@prajjwaldimri
Copy link
Contributor

@unicodeveloper This is blocked on @samsoft00.

@unicodeveloper
Copy link
Contributor

@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 🙏🏿

@Cliftonz
Copy link
Contributor

Cliftonz commented Oct 2, 2023

@samsoft00 @prajjwaldimri any updates on this?

@Cliftonz
Copy link
Contributor

Cliftonz commented Oct 6, 2023

@samsoft00 there are conflicts that we need to resolve

@solarsoft0
Copy link

@samsoft00 @prajjwaldimri any updates on this?

+1

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.

5 participants