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: automatic push click handling #403

Merged
merged 61 commits into from
Feb 12, 2024
Merged

Conversation

levibostian
Copy link
Contributor

@levibostian levibostian commented Nov 9, 2023

iOS part of ticket: https://github.com/customerio/issues/issues/11289

I suggest starting here in your PR review.

Copy link

github-actions bot commented Nov 9, 2023

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.


  • CocoaPods-FCM: levi/reliable-open-metrics (1707748160)
  • APN-UIKit: levi/reliable-open-metrics (1707748099)

@levibostian levibostian force-pushed the levi/reliable-open-metrics branch from c1da7b9 to 7a874c1 Compare November 9, 2023 19:59
@levibostian levibostian force-pushed the levi/reliable-open-metrics branch from 9d342f6 to eb9fbd9 Compare November 9, 2023 20:00
@levibostian levibostian requested a review from a team November 13, 2023 21:09
@levibostian levibostian force-pushed the levi/reliable-open-metrics branch from 1ef8e7f to 4a94c72 Compare November 15, 2023 16:50
@levibostian levibostian force-pushed the levi/reliable-open-metrics branch from 4a94c72 to a7fcc71 Compare November 15, 2023 16:54
Copy link
Contributor

@Ahmed-CIO Ahmed-CIO left a comment

Choose a reason for hiding this comment

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

the scope of these changes is massive, let's break it down and describe each change in its own right

Ahmed-Ali

This comment was marked as resolved.

@customerio customerio deleted a comment from Ahmed-Ali Jan 4, 2024
@Ahmed-CIO
Copy link
Contributor

Had a chat with Levi about this one. Given this was something took place a while ago and was already reviewed, I won't block it farther.
Moving forward though:

  • When we stack PRs and we are not using any tools that helps keep each PR in the stack reflecting its own change, then we start merge from the bottom to make sure the rest of the PRs don't end up looking like they changed the entire world.
  • Let's also avoid a situation where approved work hangs for too long, as this can cause two issues:
    • Might need to rebase on main that has conflicting changes, then resolving the conflicts with a stale PR can introduce unwanted behavior and there would be either no review, or a need to a second review that could have been avoided
    • It is better to keep the list of PRs tidy. If there is something there then it is either a draft, or something need an action.

Copy link
Contributor

@Ahmed-CIO Ahmed-CIO left a comment

Choose a reason for hiding this comment

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

Please also double check any existing new comments that took place after putting it all together and address them before landing

P.s looks like I did a review with my personal account, which definitely did not do much other than a comment :D

@levibostian levibostian marked this pull request as draft January 31, 2024 15:45
@levibostian levibostian changed the title fix: improve reliability of auto push metrics and deep link handling feat: automatic push click handling Feb 12, 2024
@levibostian levibostian marked this pull request as ready for review February 12, 2024 14:43
@levibostian levibostian merged commit 47e94c4 into main Feb 12, 2024
11 checks passed
@levibostian levibostian deleted the levi/reliable-open-metrics branch February 12, 2024 14:43
github-actions bot pushed a commit that referenced this pull request Feb 12, 2024
## [2.11.0](2.10.2...2.11.0) (2024-02-12)

### Features

* automatic push click handling ([#403](#403)) ([47e94c4](47e94c4))
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.

6 participants