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

fix(zendesk): remove endpoint global variable #1746

Merged
merged 17 commits into from
Jan 10, 2023
Merged

fix(zendesk): remove endpoint global variable #1746

merged 17 commits into from
Jan 10, 2023

Conversation

Gauravudia
Copy link
Contributor

@Gauravudia Gauravudia commented Jan 3, 2023 •
edited
Loading

Description of the change

  • Removing globally declared endpoint variable.
  • Configurations of two different users are getting interchanged during transformation because of globally declared endpoint variable

Made changes as part of the below mentioned alerts:
https://app.squadcast.com/incident/63b37b1f839f9fde8e28c756

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Fix #1

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests
  • All tests related to the changed code pass in development

Code review

  • This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • "Ready for review" label attached to the PR and reviewers mentioned in a comment
  • Changes have been reviewed by at least one other engineer
  • Issue from task tracker has a link to this pull request

@Gauravudia Gauravudia requested a review from ItsSudip January 3, 2023 11:10
@Gauravudia Gauravudia requested a review from a team as a code owner January 3, 2023 11:10
@Gauravudia Gauravudia self-assigned this Jan 3, 2023
@Gauravudia Gauravudia requested a review from mihir-4116 January 3, 2023 11:10
@sanpj2292 sanpj2292 mentioned this pull request Jan 3, 2023
Closed
10 tasks
@Gauravudia Gauravudia marked this pull request as draft January 3, 2023 11:36
@Gauravudia Gauravudia marked this pull request as ready for review January 3, 2023 12:17
@sanpj2292
Copy link
Contributor

Overall looks good. Please update the branch

sanpj2292
sanpj2292 previously approved these changes Jan 4, 2023
View reviewed changes
sanpj2292
sanpj2292 previously approved these changes Jan 5, 2023
View reviewed changes
Copy link
Contributor

@sanpj2292 sanpj2292 left a comment

Choose a reason for hiding this comment

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

LGTM

ItsSudip
ItsSudip reviewed Jan 5, 2023
View reviewed changes
Copy link
Member

@ItsSudip ItsSudip left a comment

Choose a reason for hiding this comment

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

You should add some router test cases with the error scenario.

@Gauravudia
Copy link
Contributor Author

You should add some router test cases with the error scenario.

Added

krishna2020
krishna2020 previously approved these changes Jan 5, 2023
View reviewed changes
saikumarrs
saikumarrs previously approved these changes Jan 6, 2023
View reviewed changes
@Gauravudia Gauravudia closed this Jan 6, 2023
@Gauravudia
Copy link
Contributor Author

Raised a hotfix PR for these changes here

@saikumarrs
Copy link
Member

@Gauravudia raise another PR for fixing other bugs like error handling etc., which are not part of the hotfix PR.

@ItsSudip ItsSudip reopened this Jan 9, 2023
@ItsSudip ItsSudip dismissed stale reviews from saikumarrs and krishna2020 via 63b2ba8 January 9, 2023 12:02
ItsSudip
ItsSudip previously approved these changes Jan 9, 2023
View reviewed changes
@ItsSudip ItsSudip requested a review from sanpj2292 January 10, 2023 05:46
@ItsSudip ItsSudip merged commit 836c37e into master Jan 10, 2023
@github-actions github-actions bot mentioned this pull request Jan 10, 2023
Merged
@devops-github-rudderstack devops-github-rudderstack deleted the fix.zendesk-global-variable branch April 11, 2023 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanpj2292 sanpj2292 sanpj2292 approved these changes

@ItsSudip ItsSudip ItsSudip approved these changes

@krishna2020 krishna2020 krishna2020 left review comments

@saikumarrs saikumarrs saikumarrs left review comments

@mihir-4116 mihir-4116 Awaiting requested review from mihir-4116 mihir-4116 is a code owner automatically assigned from rudderlabs/integrations

Assignees

@Gauravudia Gauravudia

Labels
bug fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Gauravudia @sanpj2292 @saikumarrs @krishna2020 @ItsSudip