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: adds option to have SDE event properties on braze event root level #16

Closed
wants to merge 4 commits into from

Conversation

spinx
Copy link
Contributor

@spinx spinx commented Jun 18, 2023

Our existing integration requires us to push Self Describing Events properties as root properties to Braze.

We also think this leads to much more intuitive usage with Braze and lessens the overhead when introducing new versions of events.

LMK if you'll be merging this so I finish this and add tests.

@adatzer
Copy link
Contributor

adatzer commented Jun 20, 2023

Hello @spinx and thank you very much for your contribution!
We are definitely keen to consider this and we will let you know as soon as possible.

Copy link
Contributor

@adatzer adatzer left a comment

Choose a reason for hiding this comment

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

Looks great @spinx ! Very happy to merge for next release!
If you have the time to add a test meanwhile, that will be awesome!

template.tpl Outdated Show resolved Hide resolved
template.tpl Outdated Show resolved Hide resolved
template.tpl Outdated Show resolved Hide resolved
@adatzer adatzer changed the base branch from main to release/0.5.0 June 21, 2023 15:22
@spinx
Copy link
Contributor Author

spinx commented Jun 21, 2023

Hi @adatzer .. I amended config options, on tests, they won't run on my environment (looks like due to CORS) and I'd need a lot of time to set that up as I don't have a dev env for this. Would you mind adding that ?

@adatzer
Copy link
Contributor

adatzer commented Jun 21, 2023

Hey @spinx ! No problem at all!
I'll cherry-pick your commit on the release branch and we can take it from there!
I'll tag you on the release PR so that you can have a look too if you wish.

Thank you very much once again! This is a very nice feature!

@adatzer
Copy link
Contributor

adatzer commented Jun 22, 2023

@spinx if you want, have a look at #20 , where everything from this PR is included. So closing this PR in favor of the release one!

@adatzer adatzer closed this Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants