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

feat: Properly demangle Swift class name #2162

Merged
merged 101 commits into from
Nov 21, 2022
Merged

feat: Properly demangle Swift class name #2162

merged 101 commits into from
Nov 21, 2022

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Sep 14, 2022

📜 Description

Add Swift code into the framework and use it to properly demangle Swift class names.

💡 Motivation and Context

see #2160

closes #1980

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

@github-actions
Copy link

github-actions bot commented Sep 14, 2022

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 1fa7dda

Makefile Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 22, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1211.33 ms 1248.42 ms 37.09 ms
Size 20.75 KiB 379.12 KiB 358.36 KiB

Previous results on branch: feat/swift

Startup times

Revision Plain With Sentry Diff
0a45bd3 1200.60 ms 1231.56 ms 30.96 ms
fd49873 1235.04 ms 1238.88 ms 3.84 ms
41b3651 1240.08 ms 1252.82 ms 12.74 ms
40e71b8 1231.78 ms 1252.52 ms 20.74 ms
e71f14d 1239.29 ms 1263.86 ms 24.57 ms
3eaa8ab 1222.88 ms 1256.54 ms 33.67 ms
d906307 1203.05 ms 1235.82 ms 32.77 ms
6ae784d 1223.80 ms 1252.58 ms 28.78 ms
762bacb 1235.85 ms 1245.48 ms 9.63 ms
0fd99e1 1238.14 ms 1239.24 ms 1.10 ms

App size

Revision Plain With Sentry Diff
0a45bd3 20.75 KiB 371.95 KiB 351.19 KiB
fd49873 20.75 KiB 379.11 KiB 358.36 KiB
41b3651 20.75 KiB 378.65 KiB 357.90 KiB
40e71b8 20.50 KiB 340.09 KiB 319.58 KiB
e71f14d 20.50 KiB 340.09 KiB 319.58 KiB
3eaa8ab 20.50 KiB 341.52 KiB 321.02 KiB
d906307 20.75 KiB 371.95 KiB 351.20 KiB
6ae784d 20.50 KiB 341.56 KiB 321.06 KiB
762bacb 20.75 KiB 371.95 KiB 351.19 KiB
0fd99e1 20.50 KiB 341.53 KiB 321.03 KiB

Copy link
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.

I'm a bit afraid of breaking the Hybrid SDKs again. How can we ensure that this is not happening?

.github/workflows/lint.yml Show resolved Hide resolved
SentryPrivate.podspec Outdated Show resolved Hide resolved
develop-docs/README.md Show resolved Hide resolved
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
Sentry.podspec Outdated Show resolved Hide resolved
@brustolin
Copy link
Contributor Author

We have two options.

  • Coordinate some tests across SDKs
  • Create an integration CI test in our repo that downloads hybrids SDK sample and try to run it.

The second one will be kind heavy thing to add into our daily process.

Copy link
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.

Let's do a beta release to mitigate the risk of breaking people. Before you merge, I think we should first coordinate with Hybrid SDKs so they are aware they need to test this. LGTM.

@philipphofmann
Copy link
Member

Let's merge this to the 8.0.0 branch once we create it. I still would like to release a hotfix with #2398 before that.

@brustolin brustolin changed the base branch from master to milestone/8.0.0 November 21, 2022 06:28
@philipphofmann philipphofmann changed the base branch from milestone/8.0.0 to 8.0.0 November 21, 2022 08:31
@brustolin brustolin merged commit a9e77dc into 8.0.0 Nov 21, 2022
@brustolin brustolin deleted the feat/swift branch November 21, 2022 08:35
kevinrenskers added a commit that referenced this pull request Nov 21, 2022
* 8.0.0:
  Update CHANGELOG.md (#2415)
  feat: Properly demangle Swift class name (#2162)
  chore: Create 8.0.0 branch
  test: include Sentry changes in hash keys (#2412)
  release: 7.31.2
kevinrenskers added a commit that referenced this pull request Nov 22, 2022
* 8.0.0:
  ref: Fix typos in OOMTracker (#2431)
  ref: Make SpanProtocol.data non nullable (#2409)
  ref: add/improve logging (#2420)
  ref: bump supported OS versions (#2414)
  test: shorten some tests (#2428)
  ref: Remove `- [SentryOptions initWithDict:didFailWithError:]` (#2404)
  ref: Mark [SpanProtocol setExtraValue:forKey] as deprecated (#2413)
  typos (#2421)
  test: Disable NSDataTracker in clearTestState (#2418)
  Update CHANGELOG.md (#2415)
  feat: Properly demangle Swift class name (#2162)
  chore: Create 8.0.0 branch
  test: include Sentry changes in hash keys (#2412)
  release: 7.31.2
  fix: Crash in Client when reading integrations (#2398)
  test: tooling improvements (#2400)
  fix: Don't increase session's error count for dropped events (#2374)
  Update CHANGELOG.md (#2396)
  release: 7.31.1
  Fix: Set the correct OOM event timestamp (#2394)

# Conflicts:
#	Sentry.xcodeproj/project.pbxproj
#	SentryPrivate.podspec
#	scripts/add-sentry-to-vlc.patch
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.

Decode Swift type names
3 participants