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

Add SDK version in format java:v{version} to the sync call #77

Merged
merged 1 commit into from
Sep 8, 2024

Conversation

KiKoS0
Copy link
Collaborator

@KiKoS0 KiKoS0 commented Sep 8, 2024

Summary

Add the SDK version to the sync request payload, assuming that we are going with java as the common denominator language : #76 (comment)

Confirming that now the language shows up as Java and the version shows up correctly in cloud mode:
image

Changes:

  • Add the SDK version and correct language.
  • Downcase the framework field.
  • Set the const field v to 0.1 as mentioned in the spec.

Checklist

  • Update documentation
  • Added unit/integration tests

Related

Copy link

linear bot commented Sep 8, 2024

Copy link
Collaborator

@albertchae albertchae left a comment

Choose a reason for hiding this comment

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

LGTM, we can change the language in a follow up if @djfarrelly decides to use a different string

@@ -174,10 +174,10 @@ class CommHandler(
private fun getRegistrationRequestPayload(origin: String): RegistrationRequestPayload =
RegistrationRequestPayload(
appName = config.appId(),
framework = framework.toString(),
sdk = "inngest-kt",
Copy link
Collaborator

Choose a reason for hiding this comment

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

interesting, I see the linear thread where you got java from. I feel like JVM or JRE is used in these situations but I can't think of a concrete example we can draw from

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah the runtime could be good too, same I know Sentry has both languages but the platform isn't auto-detected on there.

@KiKoS0 KiKoS0 merged commit c1b962d into main Sep 8, 2024
9 checks passed
@KiKoS0 KiKoS0 deleted the riadh/inn-3572-add-sdk-version-to-sync branch September 8, 2024 17:02
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.

2 participants