-
Notifications
You must be signed in to change notification settings - Fork 93
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
Merge JD's docs #147
Merged
Merge JD's docs #147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… nullable type annotations in areas where usage inferred nullability or lack-thereof.
This reverts commit 922c8c1.
tests: added chat functional test
…stead of commenting it out
JD, the merge is finally done (sorry it took so long) and the api docs are on GitHub pages Awesome work once again, thanks a thousand times for your help! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note
This is the awesome work of @JerrettDavis
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.