-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(afj): initial support for afj agent #188
feat(afj): initial support for afj agent #188
Conversation
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
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 looks great @blu3beri ! I left a question about the current structure of the new Agent
structure.
crates/agent/src/agent.rs
Outdated
pub struct Agent<A> { | ||
/// Default cloud agent structure | ||
pub agent: A, | ||
|
||
/// version string, e.g. 0.7.3 | ||
pub version: String, | ||
} |
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.
It'd be nice if we could remove the nested agent
here. I see we originally use impl TraitX
, does that no longer work? For example:
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.
That way we can just pass the instantiated agent objects around directly, or do the return types of the functions diverge too much? Enlighten me to the rusty ways.
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.
Yes you are correct, this was slightly left over from when I wanted to do a better match based on the commands... But Rust does not allow you to return Agent<A>
and Agent<B>
from seperate match arms as they are not the same type.
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Co-authored-by: Jean-Louis Leysens <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Berend Sliedrecht <[email protected]>
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.
Just left a few comments (that you have addressed already🙌). Nice job!
@jloleysens any other notes or is it ready to be merged? |
agent
key to the environmentaca-py
andafj
and if its none,aca-py
is selected