-
Notifications
You must be signed in to change notification settings - Fork 16
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 support for Ollama backend #54
base: main
Are you sure you want to change the base?
Conversation
if [ -n "$2" ] && [ "${2:0:1}" != "-" ] && [ "${2:0:3}" == "gpt" ]; then | ||
if [ -n "$2" ] && [ "${2:0:1}" != "-" ]; then | ||
if [ "${api_service}" = "openai" ] && [ "${2:0:3}" != "gpt" ]; then | ||
echo "Error: --model requires a gpt model" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the "--"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the convention of the previous error message
echo "Error: --model requires a gpt model"
or am I missing something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, nevermind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course you are right. I'm just getting started with work today ;-)
Could you also please sign your commit? |
of course! please forgive the force push |
@Dadams2 I know it’s been a while, but you’ll also need to sign off your commit. If you are still interested in getting this merged, you can easily achieve that by running git rebase -i origin/main --signoff The conflicts also need to be resolved, of course. |
These are the bare minimum changes. It might be reasonable to also include in the documentation:
-m llama3.1
I think it is somewhat safe to assume that if a user knows they want ollama they will want to specify a model themselves. This assumption is why I have omitted setting more defaults.