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 missing javadoc #2029

Merged
merged 3 commits into from Nov 5, 2023
Merged

Add missing javadoc #2029

merged 3 commits into from Nov 5, 2023

Conversation

xgouchet
Copy link
Contributor

This PR adds Javadoc/KDoc documentation on public classes, data classes, functions and enums to make it easier to use from IDEs.

Fixes #2016

Specifically:

  • JavalinConfig + all classes in the io.javalin.config package
  • Bundled plugin configs
  • EventManager
  • RequestLogger#handle, Handler#handle, ExceptionHandler#handle
  • FileRenderer#render
  • Cookie 
  • ContentType  and HandlerType enums (among others)

Note

Most documentation text was copy-pasted from the MDN Docs pages, and the Javalin documentation pages.

Copy link
Member

@zugazagoitia zugazagoitia left a comment

Choose a reason for hiding this comment

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

LGTM!
@tipsy rebase your #2006 plugin PR if you merge this to update the docs in the changed classes

Copy link
Member

@dzikoysk dzikoysk left a comment

Choose a reason for hiding this comment

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

Good job @xgouchet :) A few things that I'd address here:

  1. Some comments don't really provide useful information. I'm mainly talking about method parameters like e.g. @param contentType the content type to add on fun add(contentType: ContentType) { method. I think it's fine to just omit it & have just one simple one-liner comment about the function as a whole.
  2. Highlight that methods have default values for parameters. It might be a good idea to describe the default behavior in the docs, especially when @JvmOverloads is used.
  3. I'd reduce unnecessary spacing - empty lines & use one-liners whenever possible (Context.kt is a good visual reference here)

@xgouchet xgouchet force-pushed the xgouchet/2016/javadoc branch 3 times, most recently from 77f4c5b to 0817765 Compare November 5, 2023 09:50
@xgouchet
Copy link
Contributor Author

xgouchet commented Nov 5, 2023

I followed your suggestion @dzikoysk , and also added missing documentation on the HttpStatus enum 👍

@tipsy
Copy link
Member

tipsy commented Nov 5, 2023

This looks really good @xgouchet ! Are you ready for merge?

@xgouchet
Copy link
Contributor Author

xgouchet commented Nov 5, 2023

Yes all good for me

@dzikoysk
Copy link
Member

dzikoysk commented Nov 5, 2023

LGTM, I think these docs will help our users. It's not only describing particular functions, but also brings a general context around them :)

@tipsy tipsy merged commit 20e068c into javalin:master Nov 5, 2023
9 checks passed
@tipsy
Copy link
Member

tipsy commented Nov 5, 2023

Thank you very much @xgouchet !

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.

Revisit public API and provide missing javadocs
4 participants