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.

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

Update Trubrics action destination #2784

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Update Trubrics action destination #2784

wants to merge 12 commits into from

Conversation

guillaumefontan
Copy link

@guillaumefontan guillaumefontan commented Mar 6, 2025 •
edited
Loading

Changes

  • Added an Identify action
  • Modified the Track action to point to the new segment_track endpoint

Testing

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

@guillaumefontan guillaumefontan requested a review from a team as a code owner March 6, 2025 09:21
@brennan brennan added triaged Acknowledged by the Segment team, but not yet scheduled for review/deploy. eng-acknowledged This PR has been acknowledged by the engineering team. and removed triaged Acknowledged by the Segment team, but not yet scheduled for review/deploy. labels Mar 7, 2025
marinhero
marinhero requested changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@marinhero marinhero left a comment

Choose a reason for hiding this comment

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

Hi @guillaumefontan! I approved your PR but unfortunately CI decided to disagree with me as some of your tests are broken. Mind taking a look and letting me know when to review again? Thanks!

Screenshot 2025-03-11 at 1 14 16 PM

@guillaumefontan
Copy link
Author

@marinhero thanks for the heads up, this should be fixed now!

@jeffkayne
Copy link
Contributor

@marinhero thanks for the review. We have some customers waiting on these changes - can you let us know if this will be released on Tuesday this week? Let us know if there is anything left to do on our side to ensure that!

@jeffkayne
Copy link
Contributor

@marinhero any chance of this going out in your release today? Thanks!

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

@marinhero marinhero marinhero requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
deploy:push eng-acknowledged This PR has been acknowledged by the engineering team. mode:cloud team:external
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@guillaumefontan @jeffkayne @marinhero @brennan