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

update: update VWO SmartCode to version 2.1 #2654

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

Conversation

zeeshan-vwo
Copy link

This PR updates the VWO SmartCode in the project to version 2.1, replacing the outdated version to ensure compatibility with VWO’s latest features and improvements. The update is backward-compatible, with testing successfully completed in a local environment. No new test cases are required.

Testing

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.

  • 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.

@joe-ayoub-segment
Copy link
Contributor

Thanks for raising this PR @zeeshan-vwo .
Can you please add proof of testing?
Also would it be possible to format the new code so that it is easier to compare?

@zeeshan-vwo
Copy link
Author

Hi @joe-ayoub-segment,
We have updated the code as per request.

New unit tests are not required.

Please find the screenshot of the tests below:

image

@joe-ayoub-segment
Copy link
Contributor

hi @zeeshan-vwo thank you - I should have been clearer. It's OK that there's no unit tests, however I was asking if there's a way to demonstrate that this change won't break anything for customers who are already using the Integration.
If you're 100% confident then i can go ahead and deploy this early next week.

@zeeshan-vwo
Copy link
Author

Hi @joe-ayoub-segment, our team has already tested it in the existing integrations. You can go ahead and merge the change.

@joe-ayoub-segment
Copy link
Contributor

hi @zeeshan-vwo there are some failing CI checks
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants