Skip to content
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

Agent: add automatically generated bindings for Java #4617

Conversation

olafurpg
Copy link
Member

@olafurpg olafurpg commented Jun 19, 2024

Previously, we only generated bindings for Kotlin. The Eclipse plugin is written in Java so this PR adds support to generate Java bindings as well. Java and Kotlin are very similar so required diff is minimal. The Java code is not added to this repo because we already have the Kotlin bindings here and I don't want every agent-protocol.ts PR to have a huge diff. Instead, we can just commit the Java bindings directly to the Eclipse repo.

Note: the code generator implementation is already quite messy and it's getting more messy with this PR. I am willing to take on this tech debt for now because it's easy to catch regressions from diffs in the generated code.

Test plan

Minimal and low-risk diffs in Kotlin bindings (adding semi-colons and using Java getter method names instead of Kotlin sugar)

Previously, we only generated bindings for Kotlin. The Eclipse plugin is
written in Java so this PR adds support to generate Java bindings as
well. Java and Kotlin are very similar so required diff is minimal.

I would normally not commit the Java files into the repo but I'm failing
to run the codegen tool on Windows so I'm committing it here to make it
easy to copy-paste changes.
@olafurpg olafurpg requested review from a team June 19, 2024 07:32
@olafurpg
Copy link
Member Author

The codegen tool is only used by Eclipse at the moment (JB hasn't even adopted it yet) so this is a low-risk PR to merge

@olafurpg olafurpg enabled auto-merge (squash) June 19, 2024 07:33
@olafurpg olafurpg merged commit af9a944 into main Jun 19, 2024
21 checks passed
@olafurpg olafurpg deleted the olafurpg/cody-2312-contribute-the-cody-agent-bindings-to-the-cody-repo branch June 19, 2024 07:55
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these semicolons added?

Copy link
Member Author

Choose a reason for hiding this comment

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

so we can reuse the codegen across java/kotlin

Copy link
Contributor

@odisseus odisseus Jun 19, 2024

Choose a reason for hiding this comment

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

I don't get it. Does adding one semicolon make this a valid Java file?

P.S. Never mind. Now I see that this is a side effect of the changes in the code generator.

Copy link
Member Author

Choose a reason for hiding this comment

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

the PR has a lot of if kotlin do X else Y, and I didn't separate the kotlin/java cases when it added semicolons because kotlin understands the semicolons

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants