Classify functions exposed by Rovo modules as user invocable with testing #55
Conversation
|
Thank you for your submission! Like many open source projects, we ask that you sign our CLA (Contributor License Agreement) before we can accept your contribution. Already signed the CLA? To re-check, try refreshing the page. |
crates/forge_loader/src/manifest.rs
Outdated
| #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] | ||
| pub struct Action<'a> { | ||
| pub key: &'a str, | ||
| pub name: &'a str, // TODO: this is not in the manifest in the example forge app, wouldn't this fail? https://bitbucket.org/atlassian/rovo-data-discoverability/src/2743f6ca42589c7d1fb2d50e0d9e65e5ef1aebbe/manifest.yml#lines-3 |
There was a problem hiding this comment.
You can just leave it off, since serde will still deserialize it correctly if an object contains more keys than what you want to deserialize into.
crates/forge_loader/src/manifest.rs
Outdated
| pub struct Action<'a> { | ||
| pub key: &'a str, | ||
| pub name: &'a str, // TODO: this is not in the manifest in the example forge app, wouldn't this fail? https://bitbucket.org/atlassian/rovo-data-discoverability/src/2743f6ca42589c7d1fb2d50e0d9e65e5ef1aebbe/manifest.yml#lines-3 | ||
| pub function: &'a str, |
There was a problem hiding this comment.
This should probably be an Option<&'a str>, since the action can also just have an endpoint property instead of a function property.
crates/forge_loader/src/manifest.rs
Outdated
| // Rovo Module Functions | ||
| // No invokable functions for Rovo Agents but Action can have numerous user invokable functions | ||
| action.iter().for_each(|action| { | ||
| if let Some(function) = action.function { |
There was a problem hiding this comment.
You can just directly pass action.function to extend.
crates/forge_loader/src/manifest.rs
Outdated
|
|
||
| // TODO: Rovo Action documentation states this is mandatory field but example apps don't use it, a typo? or are "required" fields not strictly enforced? | ||
| // eg: https://bitbucket.org/atlassian/rovo-data-discoverability/src/2743f6ca42589c7d1fb2d50e0d9e65e5ef1aebbe/manifest.yml#lines-3 | ||
| pub name: &'a str, |
There was a problem hiding this comment.
If it's not mandatory or you don't know, you can just leave it off, since we really only care about function.
|
LGTM. Clean up the commit history, and I'll merge it in. |
Tests with logic of implementation still being drafted.