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

http/req: flatten #82

Closed
wants to merge 1 commit into from
Closed

Conversation

DavidLarsKetch
Copy link
Collaborator

@DavidLarsKetch DavidLarsKetch commented Jun 25, 2024

Based on @profsmallpine's feedback, this PR provides an alternative factoring of the code. In short, error handling happens in http/req.ParseBody and http/req.ParseRequestParams so this functionality is closer to the surface of the package's public methods. This removes one layer of drilling down into files in trails whenever parsing & validating behavior doesn't match with expectations.

@DavidLarsKetch
Copy link
Collaborator Author

Testing some more, I typo'd a validate struct tag and discovered validator panics. Cool. That's now handled in 1acf3e4. I'll figure out how this PR would incorporate the change if it seems like we're headed in the right direction.

In any case, I don't endorse this PR. But, I recognize the issue of nesting will be a blind spot for me as package author, so defer to everyone else. Handling those panics suggests we'll be caught out at times by our dependencies and the quality of their code. Keeping that concern below the surface and limiting its exposure to devs is desirable, in my opinion. I don't believe we all share the same level of familiarity with and comfort using schema, validator, and even json.

@DavidLarsKetch DavidLarsKetch deleted the dlk/http-req-flat branch October 23, 2024 14:57
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.

1 participant