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

Suggest to remove unnecessary dependencies #68

Open
meling opened this issue May 26, 2024 · 2 comments
Open

Suggest to remove unnecessary dependencies #68

meling opened this issue May 26, 2024 · 2 comments

Comments

@meling
Copy link

meling commented May 26, 2024

I'm considering to use this library to mock our custom wrapper around go-github. Would you consider removing the github.com/go-kit/log dependency, e.g., just print using fmt or the std lib logger?

I'm also not sure about the rationale for using the github.com/buger/jsonparser. Could the std lib json parser be used instead?

Why? Well, I've looked at the code, and the logger and json parser is only used by the command line tool. Hence, it is unfortunate that users of the mock part must bring in extra (indirect) dependencies that aren't needed for their use case.

PS: You may be interested in the v2 json-experiment that may come in a future Go release.

@migueleliasweb
Copy link
Owner

migueleliasweb commented May 26, 2024

Hi @meling , thanks for reaching out.

Would you consider removing the github.com/go-kit/log dependency, e.g., just print using fmt or the std lib logger?

The go-kit/log would be the easiest to get rid of. Using either the native logger or fmt is probably good enough.

I'm also not sure about the rationale for using the github.com/buger/jsonparser. Could the std lib json parser be used instead?

The case for the jsonparser is different. It's used to provide a dynamic parsing for the Github OpenApiSchema. Without it, although technically possible, one would have to juggle a multi-level map[string]any and given the structure, that would become messy. I'm happy to review a contributiion if you're interested to give it a shot.

PS: You may be interested in the v2 json-experiment that may come in a future Go release.

I'll keep an eye on it 👍 .

@meling
Copy link
Author

meling commented May 26, 2024

Thanks for the quick reply. Yeah, I thought there might be more to it with the jsonparser. I'm okay with keeping that, then.

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 a pull request may close this issue.

2 participants