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 all deprecation warnings & restructure targets making TelemetryDeck main one #189

Merged
merged 9 commits into from
Oct 2, 2024

Conversation

Jeehut
Copy link
Contributor

@Jeehut Jeehut commented Oct 1, 2024

Fixes #176.

Previously, when I added the new TelemetryDeck target to the package, I conservatively kept all the main code inside the old TelemetryClient target and imported TelemetryClient into TelemetryDeck. I was also hesitant to rename the package at the top level from TelemetryClient to TelemetryDeck, just to be sure not too many changes happen at once leading to too much frustration.

The outcome was that we were still using the old methods and calling them from the old ones, leading to deprecation warnings inside the package itself. In this PR, I went the next steps and moved the main logic into the new methods, so that the old methods are now calling into the new ones, making away with all the deprecation warnings. I also moved all the files from the TelemetryClient target into the TelemetryDeck target and reversed which target was importing which.

Additionally, I also renamed the package from TelemetryClient to TelemetryDeck. This should not have any consequences for Xcode users as Xcode should automatically pick up any package name changes. I've tested this in the feature/fix-warnings branch in the PirateMetrics repo. It should also not affect any Swift packages that depend on our SDK, such as server apps, because they specify the repo name SwiftSDK which hasn't changed, like so:

.product(name: "TelemetryDeck", package: "SwiftSDK")

So I believe that everything should continue to work and the hiccups for users should be minimal. But it's still worth asking for user feedback when this change is released in a future version, maybe even before on the main branch.

Note: CI was failing for visionOS due to this GitHub Actions issue, so I had to update the macOS version to 15 and then also Xcode version to 16 since Xcode 15 isn't officially supported on macOS 15.

@Jeehut Jeehut force-pushed the feature/fix-warnings branch 2 times, most recently from c0540b7 to 5240d02 Compare October 1, 2024 19:12
@Jeehut Jeehut requested a review from winsmith October 1, 2024 19:28
Copy link
Contributor

@winsmith winsmith left a comment

Choose a reason for hiding this comment

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

Love the cleanup work here! I just tested this on my machine both by including it in a project and building it myself. So satisfying to see the warnings gone!

@Jeehut Jeehut force-pushed the feature/fix-warnings branch from 5d3213d to 8a3cf0e Compare October 2, 2024 08:56
@Jeehut
Copy link
Contributor Author

Jeehut commented Oct 2, 2024

@winsmith Quickly rebased to fix the merge conflicts due to the version bump in release 2.4.0.

@Jeehut Jeehut merged commit 296d34b into main Oct 2, 2024
6 checks passed
@Jeehut Jeehut deleted the feature/fix-warnings branch October 2, 2024 09:06
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.

Get rid of all the deprecation warnings within the SDK
2 participants