-
Notifications
You must be signed in to change notification settings - Fork 761
chore: support array content values in portkey plugin checks #1223
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
Conversation
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 PR introduces valuable improvements by supporting array content values and adding robust error handling. I've identified a minor refactoring opportunity for conciseness.
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.
Approving, but I dont see you making use of contentParts in any of these plugins, we're essentially doing the same thing as getText()
but now with an added step inside the plugin to concatenate text parts, are these changes for deprecating getText() in the future?
@narengogi - getText function does not handle the new structure of messages where content can be an array of multiple parts. The new function returns the final message content parts as well as just the array of text. Depending on the plugin check, we use content part or text array as both of them have their own use cases. And yes, we need to deprecate getText soon |
Description
Motivation
Type of Change
How Has This Been Tested?
Screenshots (if applicable)
Checklist
Related Issues