-
Notifications
You must be signed in to change notification settings - Fork 65
set default to official ChatGPT model from OpenAI #122
Conversation
I think we should break something deliberately before merging this. Perhaps change the namespace of the package? |
maybe change the variable naming? |
Yeah that could do it. Something that requires user intervention. |
I like that idea! I'll implement the change shortly. |
I just noticed the env variable is optional and has a default, so a change of the variable will most likely not even be noticed by our users if they didn't change the model. |
Lets make it a required field with no default and adjust the name. |
I've also added an log entry if the new I think this is now ready for review 😃 |
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.
This looks good thanks for taking the time to make this all work. Variable name looks good and sensible to me. LGTM.
This is kind of a "dangerous" change, as the OpenAI accounts of our users will be charged by simply updating the version to this one.
I'm not sure if we should adjust the readme any further in order to make this clear, so everyone knows the usage is no longer free (and most likely will never become free again at this point).
Also closes #120 and #119