Skip to content

feat: SDKE-61 Extract and test callback logic. Part 3 #397

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Aug 25, 2025
Merged

feat: SDKE-61 Extract and test callback logic. Part 3 #397

merged 11 commits into from
Aug 25, 2025

Conversation

denischilik
Copy link
Contributor

@denischilik denischilik commented Aug 22, 2025 •
edited
Loading

Summary

This PR is second PR from the series. Previous #396

  • beginTimedEventCompletionHandler logic was extracted to separate method and covered with tests
  • logEventCallback logic was extracted to separate method and covered with tests
  • logScreenCallback logic was extracted to separate method and covered with tests
  • rewrite SettingsProviderMock in swift
  • add protocol MPDataPlanFilterProtocol
  • update check_coverage.sh to run it without creating venv
  • update schema to calculate test coverage

Testing Plan

  • Was this tested locally? If not, explain why.
  • apps were launched and checked that we receive events as expected

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

@denischilik denischilik requested a review from a team as a code owner August 22, 2025 16:50
nickolas-dimitrakas
nickolas-dimitrakas reviewed Aug 22, 2025
View reviewed changes
@nickolas-dimitrakas nickolas-dimitrakas self-requested a review August 22, 2025 18:54
nickolas-dimitrakas
nickolas-dimitrakas approved these changes Aug 22, 2025
View reviewed changes
Copy link
Contributor

@nickolas-dimitrakas nickolas-dimitrakas left a comment

Choose a reason for hiding this comment

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

LGTM, just one typo to fix

BrandonStalnaker
BrandonStalnaker approved these changes Aug 25, 2025
View reviewed changes
Copy link
Collaborator

@BrandonStalnaker BrandonStalnaker left a comment

Choose a reason for hiding this comment

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

Seems good, I'm still a little unsure about all these abstractions but I'm assuming they'll pay off more in further PRs? In the next could you add comments to explain why these methods needed to be extracted and how it improves the codebase?

@denischilik
Copy link
Contributor Author

denischilik commented Aug 25, 2025 •
edited
Loading

Seems good, I'm still a little unsure about all these abstractions but I'm assuming they'll pay off more in further PRs? In the next could you add comments to explain why these methods needed to be extracted and how it improves the codebase?

Thanks for the feedback! The rationale is already captured in the refactoring doc. Could you clarify what exactly feels unclear about these abstractions — is it why we extract exactly this methods, or something else? If it’s about that, I can highlight it more clearly in the refactoring guide or PR description.

I’m asking because it’s better to capture this clearly in the documentation.

@denischilik denischilik merged commit b6cd236 into development Aug 25, 2025
11 of 12 checks passed
@denischilik denischilik deleted the feat/SDKE-61-extract-and-test-callback-logic-3 branch August 25, 2025 17:02
@denischilik denischilik mentioned this pull request Aug 25, 2025
Merged
1 task
mparticle-automation added a commit that referenced this pull request Sep 18, 2025
@mparticle-automation
chore(release): 8.39.0 [skip ci]
804edda 
# [8.39.0](v8.38.0...v8.39.0) (2025-09-18)

### Bug Fixes

* Update Hashed Email Logic With Unassigned ([#401](#401)) ([c50b101](c50b101))

### Features

* SDKE-119 Create protocols for testability ([#402](#402)) ([12b636e](12b636e))
* SDKE-119 Create protocols for testability. Part 2 ([#403](#403)) ([6401e0c](6401e0c))
* SDKE-119 Create protocols for testability. Part 3 ([#404](#404)) ([9032458](9032458))
* SDKE-61 Extract and test callback logic. Part 1 ([#395](#395)) ([c3a7da9](c3a7da9))
* SDKE-61 Extract and test callback logic. Part 2 ([#396](#396)) ([2fb2490](2fb2490))
* SDKE-61 Extract and test callback logic. Part 3 ([#397](#397)) ([b6cd236](b6cd236))
* SDKE-61 Extract and test callback logic. Part 4 ([#398](#398)) ([3cf3c88](3cf3c88))
* SDKE-62 Refactor MPListenerController for Testability ([#400](#400)) ([cc843a5](cc843a5))
* SDKE-63-replace-logging-macros-with-swift-helpers ([#405](#405)) ([0ca4964](0ca4964))
* SDKE-64 Improve mParticle.m test coverage in swift ([#406](#406)) ([d4ef623](d4ef623))
* SDKE-64 Improve mParticle.m test coverage in swift 2 ([#407](#407)) ([b8d67ec](b8d67ec))
* SDKE-64 Improve mParticle.m test coverage in swift 3 ([#410](#410)) ([72a111c](72a111c))
@mparticle-automation
Copy link
Contributor

🎉 This PR is included in version 8.39.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrandonStalnaker BrandonStalnaker BrandonStalnaker approved these changes

@nickolas-dimitrakas nickolas-dimitrakas nickolas-dimitrakas approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@denischilik @mparticle-automation @BrandonStalnaker @nickolas-dimitrakas