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

should not ignore options.transport function provided in Sentry.init(...) #2398

Merged
merged 6 commits into from
Jul 28, 2022

Conversation

zivl
Copy link
Contributor

@zivl zivl commented Jul 27, 2022

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

I've updated the ReactNativeClient constructor to first check if options.transport is provided and if so - then it takes it and calls to super(options). If options.transport not provided then keep the previous logic and conduct react-native transport layer and then calls to super(options).

💡 Motivation and Context

this resolves issue #2397

💚 How did you test it?

I've added a test which gets the given custom transport layer and then runs and validates the customized send and flush functions.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

🔮 Next steps

test/client.test.ts Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@zivl please add the changelog entry.
Thank you for fixing this.

@zivl zivl changed the title Fix issue 2397 should not ignore options.transport function provided in Sentry.init(...) Jul 27, 2022
@zivl
Copy link
Contributor Author

zivl commented Jul 27, 2022

@marandaneto change requests are done. please review again

@marandaneto
Copy link
Contributor

@marandaneto change requests are done. please review again

Awesome.
Double-check the linter.

/home/runner/work/sentry-react-native/sentry-react-native/test/client.test.ts
  118:14  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`  @typescript-eslint/unbound-method
  11[9](https://github.com/getsentry/sentry-react-native/runs/7539677797?check_suite_focus=true#step:5:10):14  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`  @typescript-eslint/unbound-method

When fixing this, we are ready to LGTM.

@zivl
Copy link
Contributor Author

zivl commented Jul 27, 2022

@marandaneto I can't get out of it without really put a eslint-ignore... comment as this is a dummy implementation for the sake of the test. it is also stated here that it is better to just add an ignore comment, can you go with that? or have any other solution?

@marandaneto
Copy link
Contributor

marandaneto commented Jul 27, 2022

@marandaneto I can't get out of it without really put a eslint-ignore... comment as this is a dummy implementation for the sake of the test. it is also stated here that it is better to just add an ignore comment, can you go with that? or have any other solution?

We can eslint-ignore it, no problem.

@marandaneto marandaneto enabled auto-merge (squash) July 28, 2022 06:34
@marandaneto marandaneto merged commit abcd022 into getsentry:main Jul 28, 2022
@zivl zivl deleted the fix-issue-2397 branch July 28, 2022 06:36
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