chore: remove needless clone() in goose/providers #2528
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seddonm1
changed the title
cgwalters
approved these changes
Jun 8, 2025
Contributor
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
It looks like this project is pretty buried in unreviewed PRs. I just added some more 😄 so I'm trying to pay things down and review other people's PRs. Feel free to review mine too...or someone else's.
Anyways I have to say I gave this PR a review and it looks good to me! Definitely one of those cases where "if it compiles it works".
Collaborator
|
@seddonm1 thanks, we're getting caught up can you resolve conflicts and we'll take another look! |
Collaborator
|
@seddonm1 yeah thanks - I think it makes sense - do you mind resolving conflicts/updating, and if you find anywhere else lets do that as well |
michaelneale
approved these changes
Jun 18, 2025
Collaborator
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
it you can update/resolve conflicts and once this builds, we can merge it in.
seddonm1
force-pushed
the
chore-remove-needless-clone
branch
from
5a5548f to
842e5ca
Compare
Contributor
Author
|
Rebased. Should be good to go. |
jamadeo
approved these changes
Jul 3, 2025
Collaborator
|
ugh and then we didn't. If you can give it one more try, I'll land it. Sorry! |
seddonm1
force-pushed
the
chore-remove-needless-clone
branch
from
7278061 to
3c59f24
Compare
Contributor
Author
|
@DOsinga rebased and removed a few more needless clone() |
Collaborator
|
excellent - can you do the DCO thing though? https://github.com/block/goose/pull/2528/checks?check_run_id=46131682554 Thanks so much! |
seddonm1
force-pushed
the
chore-remove-needless-clone
branch
from
3c59f24 to
5e6928b
Compare
Contributor
Author
|
No worries @DOsinga . Squashed, signed and good to go. |
Contributor
Author
|
Sorry lint failed. Will fix tomorrow. |
seddonm1
force-pushed
the
chore-remove-needless-clone
branch
from
5e6928b to
3f6f994
Compare
Signed-off-by: Mike Seddon <[email protected]>
seddonm1
force-pushed
the
chore-remove-needless-clone
branch
from
3f6f994 to
759c131
Compare
Contributor
Author
|
@DOsinga rebased (to include new litellm provider) and everything should work this time 😅 |
katzdave
added a commit
that referenced
this pull request
Jul 22, 2025
* 'main' of github.com:block/goose: (23 commits) fix: add fallback id to messages if none provided (#3584) feat: migrate ErrorData from internal mcp crates to rmcp version (#3586) fix: adjust subrecipe description to allow running tests (#3585) Scenario tests (#3430) feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564) Restore recipe parameters functionality (#3530) feat: Enhanced loading states with thinking icons and flying bird animation (#3478) Agent loop defensive (#3554) chore: remove needless clone() in goose/providers (#2528) Add recipe install warning (#3537) Replace mcp_core::resource::* with rmcp types (#3563) Add YouTube video embed to using-goosehints.md (#3560) fix: ensure retry-config and success-criteria are populated in openapi spec (#3575) fix: use sequential when sub recipe task is 1. (#3573) fix: track message id to keep like with like (#3572) Replace mcp_core::prompt with rmcp::model types (#3561) feat (ui): close recipe modals with esc key (#3568) feat: recipes can retry with success criteria (#3474) Env var to set Ollama request timeout (#3516) Updating docs to match new UI (#3552) ...
michaelneale
added a commit
that referenced
this pull request
Jul 23, 2025
* main: docs: desktop recipe format (#3594) Fix model display name not being updated immediately after leaving settings (#3587) Added option to summarize the chat when an error is triggered (#3598) Remove mcp_macros and unused types (#3581) fix: add fallback id to messages if none provided (#3584) feat: migrate ErrorData from internal mcp crates to rmcp version (#3586) fix: adjust subrecipe description to allow running tests (#3585) Scenario tests (#3430) feat: migrate JsonRpcMessage/Request/Response/Error/Notification from internal mcp crates to rmcp versions (#3564) Restore recipe parameters functionality (#3530) feat: Enhanced loading states with thinking icons and flying bird animation (#3478) Agent loop defensive (#3554) chore: remove needless clone() in goose/providers (#2528) Add recipe install warning (#3537) Replace mcp_core::resource::* with rmcp types (#3563) Add YouTube video embed to using-goosehints.md (#3560) fix: ensure retry-config and success-criteria are populated in openapi spec (#3575)
atarantino
pushed a commit
to atarantino/goose
that referenced
this pull request
Aug 5, 2025
Signed-off-by: Mike Seddon <[email protected]> Signed-off-by: Adam Tarantino <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have been reading through the code of goose and found that there are a lot of needless
.clone()calls that probably the compiler removes but makes the code harder to read as it does have a semantic difference.I am not sure if you would like this sort of PR but I can do more of these as I go through the code.