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

Add OpenAI Plugin Example #10

Merged
merged 31 commits into from
May 14, 2024
Merged

Add OpenAI Plugin Example #10

merged 31 commits into from
May 14, 2024

Conversation

vulus98
Copy link
Contributor

@vulus98 vulus98 commented May 12, 2024

  • Address the latest comments on the README

Copy link
Contributor

@davidoort davidoort left a comment

Choose a reason for hiding this comment

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

Please rebase on master and make sure lib files are gitignored

examples/openai/README.md Show resolved Hide resolved
plugins/openai/README.md Outdated Show resolved Hide resolved
@vulus98
Copy link
Contributor Author

vulus98 commented May 13, 2024

Filled in the readme both for plugin and for examples

@vulus98 vulus98 requested a review from davidoort May 13, 2024 08:39
Copy link
Contributor

@davidoort davidoort left a comment

Choose a reason for hiding this comment

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

Great work on the docs!

I added some small comments here and there, and don't forget to address the old ones 🙏

Did not dig too deep into the individual examples because it doesn't need to be perfect for now

examples/openai/.gitignore Outdated Show resolved Hide resolved
examples/openai/README.md Outdated Show resolved Hide resolved
Comment on lines 297 to 298
## Contributing
If you want to contribute to the OpenAI plugin, you can link to a local instance of the plugin by running `npm link` in the plugin directory and `npm link genkitx-openai-plugin` in the examples directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

in order to prevent this from getting buried, I'd consider putting the usage examples in a separate doc that gets linked in the examples/openai/README.md file

This will also be useful if we move to a structure where we have examples for all plugins in one package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find the markdown support for it, only some external tools that allow that. If I got you right, you want the same content, but would like to separate the usage examples in the new markdown file which gets imported in our README.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or to omit the examples in the main README and just link to other .md file which will contain them?

Copy link
Contributor

@davidoort davidoort May 13, 2024

Choose a reason for hiding this comment

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

Why not: [Usage examples OpenAI Plugin](docs/usage_openai.md)?
I'd be in favor of the latter. so we can link to docs/usage_openai.md and other ones. lmk if this makes sense

plugins/openai/README.md Outdated Show resolved Hide resolved
plugins/openai/README.md Outdated Show resolved Hide resolved
@vulus98 vulus98 changed the title Adding some OpenAI plugin examples Add OpenAI Plugin May 13, 2024
@davidoort davidoort changed the title Add OpenAI Plugin Add OpenAI Plugin Example May 13, 2024
Copy link
Contributor

@davidoort davidoort left a comment

Choose a reason for hiding this comment

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

small things

.gitignore Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/src/index.ts Outdated Show resolved Hide resolved
plugins/openai/src/gpt.ts Outdated Show resolved Hide resolved
plugins/openai/src/gpt.ts Outdated Show resolved Hide resolved
@vulus98
Copy link
Contributor Author

vulus98 commented May 13, 2024

Check currently fail as it checks if the genkit version is 11, but we updated it to 12 in the code. I'll leave it to you to change this test and be aware of this change before merging

@davidoort davidoort merged commit 7264c49 into main May 14, 2024
1 check passed
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.

3 participants