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

Add basic docstrings for Application and Request #2626

Merged
merged 2 commits into from
May 21, 2021

Conversation

kmahar
Copy link
Contributor

@kmahar kmahar commented May 20, 2021

This adds inline documentation for Application and Request. This helps when trying to integrate Jazzy (realm/jazzy#936)

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thank you!

@0xTim 0xTim changed the title Add basic docstrings for Application and Request Add basic docstrings for Application and Request May 21, 2021
@0xTim
Copy link
Member

0xTim commented May 21, 2021

@kmahar I'm assuming this needs an actual release so you can pull in the updated dependency with the docs?

@0xTim
Copy link
Member

0xTim commented May 21, 2021

Original comment:

I'm trying to include documentation for some extensions I've added to these types in API docs generated with Jazzy, and unfortunately due to realm/jazzy#936 they won't show up unless if these types have their own docstrings to begin with.

I can hack around it in my docs generation script, but it seemed like a better fix to at least provide some very basic docstrings on these types in Vapor itself. (The best fix would probably be fixing the Jazzy bug, but I don't know Ruby at all 🙂)

In this basic form I guess the docstrings don't provide a lot of value, but they seem like a fine starting point to provide more useful documentation in the future, and since both extending these types and using Jazzy are fairly common, I figured others might benefit from this change as well.

@0xTim 0xTim added the semver-patch Internal changes only label May 21, 2021
@kmahar
Copy link
Contributor Author

kmahar commented May 21, 2021

@0xTim Yeah, we would need a release. That said, I can work around it in the short term by manually editing the generated docs to add in the content, so it's not super urgent, just nice to have for the future 🙂

@0xTim
Copy link
Member

0xTim commented May 21, 2021

Cool I'll do a release. Yay for automation!

@0xTim 0xTim merged commit d6605a6 into vapor:main May 21, 2021
@VaporBot
Copy link
Contributor

These changes are now available in 4.45.5

@kmahar
Copy link
Contributor Author

kmahar commented May 21, 2021

Awesome, thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Internal changes only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants