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: Datadog integration leads to build failures - WPB-11936 #2153

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

netbe
Copy link
Collaborator

@netbe netbe commented Nov 13, 2024

TaskWPB-11936 Re-enable CallQualityControllerTests

This reverts commit 465f8be.

Issue

This PR attempts to fix develop tests by:

  1. reverting PR fix: restore CallQualityControllerTests - WPB-11936 #2110: this is still an issue we will need to fix (ticket back to TODO)

  2. separate WireDatadog and WireAnalytics:

WireAnalytics is still in SynEngine:
WireDatadog is in WireCommonComponents
The canImport is removed entirely from WireCommonComponents so all datadog imports are within WireDatadog

  1. disable code coverage for SyncEngine

Testing

@echoes-hq echoes-hq bot added the echoes: technical-roadmap/technical-debt Changes intended at mitigating risks label Nov 13, 2024
Copy link
Contributor

github-actions bot commented Nov 13, 2024

Test Results

    3 files    447 suites   5m 51s ⏱️
3 123 tests 3 123 ✅ 0 💤 0 ❌
3 131 runs  3 131 ✅ 0 💤 0 ❌

Results for commit d6b2dab.

♻️ This comment has been updated with latest results.

@netbe netbe added the WIP label Nov 13, 2024
@netbe netbe changed the title revert: restore CallQualityControllerTests - WPB-11936 fix: DataDog integration leads to build failures - WPB-11936 Nov 15, 2024
@netbe netbe changed the title fix: DataDog integration leads to build failures - WPB-11936 fix: Datadog integration leads to build failures - WPB-11936 Nov 15, 2024
@netbe netbe requested review from a team, samwyndham, El-Fitz, KaterinaWire and caldrian and removed request for a team November 15, 2024 15:56
@netbe netbe removed the WIP label Nov 15, 2024
Copy link
Contributor

@samwyndham samwyndham left a comment

Choose a reason for hiding this comment

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

🥳

.product(name: "DatadogTrace", package: "dd-sdk-ios")
],
dependencies: datadogDependencies(),
path: "Sources/WireDatadog",
Copy link
Contributor

@samwyndham samwyndham Nov 15, 2024

Choose a reason for hiding this comment

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

Did you consider making the path change instead of the source files? It might be easier to maintain 🤔

As a side note, if we take the file route perhaps this line is not necessary as I guess that is the default path

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

from the doc it was saying here lies the directory and on the other one just filenames. I didn't play with this

@@ -9,6 +9,7 @@
}
],
"defaultOptions" : {
"codeCoverage" : false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you confirm that this was necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not completely sure, but I think it does not hurt to keep it disabled for now since we don't collect code coverage for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: technical-roadmap/technical-debt Changes intended at mitigating risks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants