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

Updated all examples to latest OpenAI package with new syntax, plus other upgrades #1

Merged
merged 24 commits into from
Feb 23, 2024

Conversation

sxflynn
Copy link
Contributor

@sxflynn sxflynn commented Feb 9, 2024

Here are some of the major changes in this PR:

  • upgraded the OpenAI package to 1.x which has completely new syntax
  • Replaced all Create React App examples with Vite see explanation from React team
  • Switched to adding .env support in lieu of system environment variables to make it more beginner friendly along with a .env.example file

Minor quality of life enhancements

  • upgraded all requirements.txt packages to the latest versions, and tested
  • added a FastAPI startup decorator in /03_api to help someone try out a GET request with one click to the browser
  • added more setup info in the readme to help beginners, like setting up environment variables
  • cleaned up .gitignore inconsistences
  • cleaned up random readmes in the various React folders

Copy link
Owner

@developing-human developing-human left a comment

Choose a reason for hiding this comment

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

Really appreciate your help in bringing things up to date. I made a few minor comments.

In the future (here and on other projects), more bite sized PRs are usually easier to review (and revert, if anything goes wrong). For example, this one may have been broken into one for each of: moving to vite, upgrading openai usage, introducing dotenv, upgrading packages, and other cleanup.

Again, thank you for helping bring things up to date.

03_api/api.py Show resolved Hide resolved
03_api/api.py Outdated Show resolved Hide resolved
Comment on lines +63 to +64
- Starting with step 3, start the python backend with `uvicorn api:app --reload`
- (For 4-6) run `npm install` then `npm run dev`
Copy link
Owner

Choose a reason for hiding this comment

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

I'd prefer the same phrasing between "Starting with step 3" and "(For 4-6)" but no preference as to which format is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you'd prefer the original phrasing (but with npm run dev) then that's fine. I can remove the "Starting with step 3, " text

05_streaming/index.html Outdated Show resolved Hide resolved
Copy link
Contributor Author

@sxflynn sxflynn left a comment

Choose a reason for hiding this comment

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

Thanks for your feedback. I'll look into the best workflow for taking a whole bunch of commits and breaking them down into separate PRs, because you're right that PRs should have a single focus.

03_api/api.py Show resolved Hide resolved
Comment on lines +63 to +64
- Starting with step 3, start the python backend with `uvicorn api:app --reload`
- (For 4-6) run `npm install` then `npm run dev`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you'd prefer the original phrasing (but with npm run dev) then that's fine. I can remove the "Starting with step 3, " text

@developing-human developing-human merged commit 46c681e into developing-human:main Feb 23, 2024
@sxflynn sxflynn deleted the updates branch March 31, 2024 04:34
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.

2 participants