Skip to content

Conversation

@rakyll
Copy link
Member

@rakyll rakyll commented Nov 11, 2025

  • Rename server/restapi to server/adkrest to match server/adka2a.
  • Move non-public packages to internal.
  • Rename server/handlers to server/controllers because it contains controllers.
  • Name all handlers *Handler per Go convention.
  • Move AgentLoader to the agent package.

TODO: Consider moving controllers into an internal package.

@rakyll rakyll force-pushed the rest branch 4 times, most recently from 529aa8b to b010c25 Compare November 11, 2025 05:37
@rakyll rakyll marked this pull request as ready for review November 11, 2025 05:38
@rakyll rakyll requested a review from dpasiukevich November 11, 2025 05:40
@rakyll
Copy link
Member Author

rakyll commented Nov 12, 2025

PTAL

@dpasiukevich
Copy link
Collaborator

LGTM!

Let me know if you'd like to update this PR to hide restapi entities and expose only NewHandler(config *launcher.Config) http.Handler

@rakyll
Copy link
Member Author

rakyll commented Nov 12, 2025

I'll revisit this PR to make some changes the hide more of the public surface. I'll introduce something like NewHandler as a part of the change.

@rakyll rakyll force-pushed the rest branch 10 times, most recently from 5797441 to f094ad0 Compare November 12, 2025 19:19
@rakyll
Copy link
Member Author

rakyll commented Nov 12, 2025

PTAL

@rakyll
Copy link
Member Author

rakyll commented Nov 12, 2025

We will follow up with some more changes to the launchers, but this PR is ready for review.

@kdroste-google
Copy link
Collaborator

adk-docs /examples/go should be updated after we merge: for sure cloud-run and a2a_basic/remote_a2a/check_prime_agent, maybe more

@kdroste-google kdroste-google self-requested a review November 13, 2025 12:52
kdroste-google
kdroste-google previously approved these changes Nov 13, 2025
Copy link
Collaborator

@kdroste-google kdroste-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One extra file (eval.go) to be deleted if not needed.

@rakyll
Copy link
Member Author

rakyll commented Nov 13, 2025

Can I have LGTM again? I had to rebase.

@rakyll rakyll merged commit f5d265a into main Nov 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants