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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add a new enableAutoBreadcrumbTracking option #1958

Merged
merged 6 commits into from
Jul 11, 2022

Conversation

kevinrenskers
Copy link
Contributor

@kevinrenskers kevinrenskers commented Jul 11, 2022

馃摐 Description

We're unifying the options, so breadcrumb tracking is also enabled via a boolean option enableAutoBreadcrumbTracking.

馃挕 Motivation and Context

Closes #427.

馃挌 How did you test it?

Unit test, also sample app.

馃摑 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

馃敭 Next steps

@github-actions
Copy link

github-actions bot commented Jul 11, 2022

Fails
馃毇 Please consider adding a changelog entry for the next release.
Messages
馃摉 Do not forget to update Sentry-docs with your feature once the pull request gets approved.
`Instructions and example for changelog`$

Please add an entry to CHANGELOG.md` to the "Unreleased" section under the following heading:

To the changelog entry, please add a link to this PR (consider a more descriptive message):`

- Add a new enableAutoBreadcrumbTracking option(#1958)

If none of the above apply, you can opt out by adding _#skip-changelog_ to the PR description.

Generated by 馃毇 dangerJS against 2090a79

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Only a few missing tests. Thanks 馃憣

CHANGELOG.md Outdated Show resolved Hide resolved
Tests/SentryTests/SentryOptionsTest.m Show resolved Hide resolved
@@ -19,6 +22,13 @@ @implementation SentryAutoBreadcrumbTrackingIntegration

- (void)installWithOptions:(nonnull SentryOptions *)options
{
if (!options.enableAutoBreadcrumbTracking) {
Copy link
Member

Choose a reason for hiding this comment

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

m: Please add a test for this new functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still a draft PR 馃槄

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but I didn't see anything apart from the missing tests, so I added a comment to not forget about it.

@kevinrenskers
Copy link
Contributor Author

Do we need to update any docs for this?

@kevinrenskers kevinrenskers marked this pull request as ready for review July 11, 2022 11:40
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 馃檹

@kevinrenskers kevinrenskers merged commit db38c00 into master Jul 11, 2022
@kevinrenskers kevinrenskers deleted the feat/427-enableAutoBreadcrumbTracking branch July 11, 2022 12:32
kevinrenskers added a commit that referenced this pull request Jul 11, 2022
* master:
  feat: Add a new enableAutoBreadcrumbTracking option (#1958)
  fix: Don't send error 429 as network_error (#1957)
  build: Format code only on specific files (#1956)
  Update SentryANRTrackerTests.swift (#1955)
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.

enableAutoSessionTracking API
2 participants