-
-
Notifications
You must be signed in to change notification settings - Fork 498
Suggest a compatible loader plugin for when one is not installed and a model is selected #1167
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
src/renderer/components/Experiment/Foundation/RunModelButton.tsx
Outdated
Show resolved
Hide resolved
src/renderer/components/Experiment/Foundation/RunModelButton.tsx
Outdated
Show resolved
Hide resolved
…ab-app into fix/run-button-compatible-message
…m/transformerlab/transformerlab-app into fix/run-button-compatible-message
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| }, [data, supportedEngines]); | ||
|
|
||
| // Use the suggested loader plugin from API (platform-aware) | ||
| const compatibleLoaderPlugins = suggestedLoaderPlugin |
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.
Nitpick: Probably don't need to create and convert this to an array just to call [0] on it later?
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 kept it as an array so we can show more of them later on if we wanted and change the messaging. And it also helped with the length condition. Should I not do that and just return the first one and check for a blank string then?
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.
Yeah I htink with the change to just get a single suggestion from the backend it makes more sense to get rid of the array for now.
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.
Fixed
Fixes #1020