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: add convenience functions for summary #284

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

xarunoba
Copy link
Contributor

@xarunoba xarunoba commented Mar 5, 2024

This extends the convenience functions further by generating phrases and sentences for Summary and Description respectively.

I didn't want to add further dependencies like cases and whatnot since I'm not sure if you're fine with that or not. This looks a bit hacky but it does the trick for a basic implementation.

Tell me what you think :)

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.04%. Comparing base (8d59852) to head (c78bfb9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   95.03%   95.04%   +0.01%     
==========================================
  Files          18       18              
  Lines        2717     2726       +9     
==========================================
+ Hits         2582     2591       +9     
  Misses         98       98              
  Partials       37       37              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danielgtaylor
Copy link
Owner

@xarunoba thanks for the PR! I think adding the summary is useful since it'll show up in the docs instead of just the URL, and it's sort of reasonable as that's how you would describe the URL given no other info. The description I'm not so sure about though - what does this give us? It seems to just be the same thing without adding any value. I think if people want more than a generated ID + summary then they should just use huma.Register for full control. What do you think?

I'm happy to merge if you remove the description and then document the generated summary behavior and how to override it, see https://github.com/danielgtaylor/huma/blob/main/docs/docs/features/operations.md#convenience-methods for where to edit. Thanks!

@xarunoba
Copy link
Contributor Author

xarunoba commented Mar 7, 2024

@danielgtaylor hmm I agree with your standpoint. I just included description for the sake of why not (aha)
I have removed the description and added the documentation about the generated summary behavior and how to override it. :)

Copy link
Owner

@danielgtaylor danielgtaylor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 👍

@danielgtaylor danielgtaylor merged commit 77be0e2 into danielgtaylor:main Mar 7, 2024
3 checks passed
@xarunoba xarunoba changed the title feat: add convenience functions for summary and description feat: add convenience functions for summary Mar 8, 2024
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

2 participants