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

fix: use typemap in JVM #2556

Merged
merged 1 commit into from
Sep 1, 2024
Merged

fix: use typemap in JVM #2556

merged 1 commit into from
Sep 1, 2024

Conversation

stuartwdouglas
Copy link
Collaborator

fixes: #2513
fixes: #2510

@stuartwdouglas stuartwdouglas requested review from a team and jvmakine and removed request for a team August 29, 2024 21:36
This was referenced Aug 29, 2024
@stuartwdouglas stuartwdouglas added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Aug 29, 2024
go.mod Outdated
@@ -47,6 +47,7 @@ require (
github.com/santhosh-tekuri/jsonschema/v5 v5.3.1
github.com/sqlc-dev/pqtype v0.3.0
github.com/swaggest/jsonschema-go v0.3.72
github.com/tbd54566975/web5-go v0.24.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this was added just to get a type to use for testing, but this will slow down the builds, so let's remove it and use some type in the stdlib or something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it ok to keep using it in the testdata project (so it is not in the main go.mod, just the test project used in integration tests).

Using this class actually showed up some real world issues as it has a custom serialization format, and was supported in both languages via third party libraries.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah that's fine!

@stuartwdouglas stuartwdouglas force-pushed the stuartwdouglas/2510 branch 2 times, most recently from 7459ffe to 9d67773 Compare August 30, 2024 13:44
@alecthomas alecthomas added this pull request to the merge queue Aug 31, 2024
@alecthomas alecthomas removed this pull request from the merge queue due to a manual request Aug 31, 2024
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Aug 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 31, 2024
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Aug 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 31, 2024
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Sep 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 1, 2024
@stuartwdouglas stuartwdouglas added this pull request to the merge queue Sep 1, 2024
Merged via the queue into main with commit 58ce5a0 Sep 1, 2024
87 checks passed
@stuartwdouglas stuartwdouglas deleted the stuartwdouglas/2510 branch September 1, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type aliases for JVM Codegen'd Kotlin clients don't use ftl:typemap annotations
2 participants