-
Notifications
You must be signed in to change notification settings - Fork 82
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
Added Documentation, Updated Access Modifiers, and Corrected Null Checking/Assertions #142
Added Documentation, Updated Access Modifiers, and Corrected Null Checking/Assertions #142
Conversation
… nullable type annotations in areas where usage inferred nullability or lack-thereof.
This reverts commit 922c8c1.
@awaescher Whoops! I was tinkering a bit with the website build. Reverted the static includes. The github action should handle all of this in its own branch without polluting anything else. |
Feels way better now. Wow, this seems to be really cool - I'll let you tinker further as long as it is a draft, okay? |
tests: added chat functional test
@awaescher I believe I've gotten the documentation into a workable state. The demo site has been updated. |
Outstanding work, thank you so much for the work you put into this! |
All good with me! I definitely need a second set of eyes to review and make sure I didn't make something |
@awaescher I think it's all good from my end now. Please let me know if you have any questions or change requests. I do want to expand the documentation further, but there's probably more than enough in this PR already. |
I am on the road today, I will have another look at it tomorrow or the day after tomorrow. |
…stead of commenting it out
I merged it to a local branch, so that I can easiy check it out locally. |
As mentioned in #113, it would be nice to have some sort of auto-generated documentation site for the library's API.
I updated some of the in-code documentation and created a Demo Site.
While reviewing the generated site, I altered the visibility of several classes to be internal. They appeared to largely be internal constructs only used by the OllamaSharp library itself.
Document Site
Demo Site Home Page:
Demo Site - Custom Docs
Demo Site - Auto Generated API Docs
The documentation is being generated using docfx. It is highly configurable, and should meet a wide variety of use cases. I've done around the minimum amount of configuration necessary to get it up and running. I did add a plugin to make handing layouts a bit easier, and with that, I also added some CSS styles to help create more sophisticated layouts.
I wasn't sure what to use for the icon, and I didn't want to create confusion by using the official Ollama logo throughout. I created a simple, silly .NETish themed Ollama logo derivative. Please feel free to replace this with whatever appropriate branding you deem fit. I wanted to make sure it wasn't the default.
Class Access Modifiers
To minimize the scope of the documentation, I went through and adjusted several classes to have different access modifiers. I reduced their scope from public down to internal. The modified classes should be audited, and if they shouldn't be internal, it would be useful to add documentation around how one could extend those pieces of the library.
Documentation
I updated documentation throughout the library. In most of the places, the changes were limited to: adding documentation for return types, adding
<see ref="">
and<see href="">
to relevant documentation, adding example code snippets, and clarifying verbiage.Nullable Annotations
I noted several places where null-checking, null-asserting, or null reference type usage was inconsistent. In these locations, I changed the null-annotations by adding or removing a
!
or?
to the affected code. In at least one area, I added a more explicit exception where a null could be passed into an area where one is not permitted.