-
Notifications
You must be signed in to change notification settings - Fork 284
[Taguchi destinations][add-units-test][fix-bug] #3139
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
[Taguchi destinations][add-units-test][fix-bug] #3139
Conversation
Hi @kc-ong-taguchi thanks for the PR. |
Hi Joe,
It’s because I pull the commits from the main branch as well and not because I made any changes to other files beside Taguchi folder. Can you update and sync the Taguchi-destinations branch with the main branch commits? Thanks.[image.png]
From: Joe Ayoub ***@***.***>
Date: Tuesday, 5 August 2025 at 9:18 PM
To: segmentio/action-destinations ***@***.***>
Cc: KC Ong ***@***.***>, Mention ***@***.***>
Subject: #External Email:Re: [segmentio/action-destinations] [Taguchi destinations][add-units-test][fix-bug] (PR #3139)
[https://avatars.githubusercontent.com/u/45374896?s=20&v=4]joe-ayoub-segment left a comment (segmentio/action-destinations#3139)<#3139 (comment)>
Hi @kc-ong-taguchi<https://github.com/kc-ong-taguchi> thanks for the PR.
Looks like the PR made changes to files other than the Taguchi files.
Could you correct this please?
—
Reply to this email directly, view it on GitHub<#3139 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BL7KXZU2HPNURBSOLUYXRYT3MCVLFAVCNFSM6AAAAACDEUSA5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNJVGE4DINZZG4>.
You are receiving this because you were mentioned.
|
Hi @kc-ong-taguchi I've updated the branch from main. |
Hi @kc-ong-taguchi - that didn't seem to work. |
I'll close this PR for now. |
ee5cbcc
to
dec575c
Compare
Hi @joe-ayoub-segment, the commits look good and clean now. |
Hi @joe-ayoub-segment , any update on when this PR will be merged? |
type: 'number', | ||
required: false | ||
}, | ||
products: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kc-ong-taguchi ,
can we move products into its own field please?
products is an object array field, so will not render correctly if contained within another object field.
Thanks,
Joe
Replaced with this PR #3139 I'll deploy on Tuesday. |
A summary of your pull request, including the what change you're making and why.
syncEvent
to sync all event related to the audiences dataTesting
Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.