Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

fix: envelope header has sdkVersion and not sdkInfo#488

Merged
marandaneto merged 4 commits into
getsentry:masterfrom
marandaneto:fix/sdk-envelope-header
Jul 21, 2020
Merged

fix: envelope header has sdkVersion and not sdkInfo#488
marandaneto merged 4 commits into
getsentry:masterfrom
marandaneto:fix/sdk-envelope-header

Conversation

@marandaneto
Copy link
Copy Markdown
Contributor

@marandaneto marandaneto commented Jul 20, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

fix: envelope header has sdkVersion and not sdkInfo

💡 Motivation and Context

getsentry/develop#89
docs were wrong about which object to use

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

@marandaneto marandaneto requested a review from a team July 20, 2020 11:06
Copy link
Copy Markdown
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Good job 👏 , I added a few comments.

Comment thread sentry-core/src/main/java/io/sentry/core/protocol/SdkVersion.java
Comment thread sentry-android-core/src/test/java/io/sentry/android/core/AndroidSerializerTest.kt Outdated
Comment thread sentry-android-ndk/src/main/java/io/sentry/android/ndk/SentryNdk.java Outdated
Comment thread sentry-core/src/main/java/io/sentry/core/SentryEnvelopeHeaderAdapter.java Outdated
Comment on lines +50 to +51
// TODO: sentry-core should add itself as the version may mismatch
sdkVersion.addPackage("maven:sentry-core", version);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM 🌞

@marandaneto marandaneto merged commit 89da1a3 into getsentry:master Jul 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants