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

Migrating from Autopilot to Ortto #69

Merged
merged 20 commits into from
Feb 8, 2024

Conversation

RamRamez
Copy link
Collaborator

@RamRamez RamRamez commented Jan 30, 2024

#1184

Copy link
Collaborator

@jainkrati jainkrati left a comment

Choose a reason for hiding this comment

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

Looks good to me, please test thoroughly before merging

Copy link
Contributor

@mohammadranjbarz mohammadranjbarz left a comment

Choose a reason for hiding this comment

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

Thanks @RamRamez great work
I commented some changes and write some of them here:

  1. Remove body.sendSegment, the sendEmail field is enough if we just want to know we should send email or not
  2. Change schemaValidator.ts to remove sendSegment and segment from schema validators and add emailPayload field
  3. Change SegmentAndSchemaValidator , remove segment, add emailPayload and define schema for those validators

If you had any question about these items plz DM me to talk about it ASAP

@RamRamez RamRamez marked this pull request as ready for review February 8, 2024 12:03
Copy link
Contributor

@mohammadranjbarz mohammadranjbarz left a comment

Choose a reason for hiding this comment

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

Thanks @RamRamez LGTM

@mohammadranjbarz mohammadranjbarz merged commit 1fd9abe into staging Feb 8, 2024
3 checks passed
@mohammadranjbarz mohammadranjbarz deleted the migrating-from-autopilot-to-ortto branch February 8, 2024 12:29
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.

3 participants