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

[SDK-2527] Add request & response objects to the BranchLogCallback #1458

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

nsingh-branch
Copy link
Contributor

Reference

SDK-2427 -- Add request & response objects to the BranchLogCallback

Summary

Updates the BranchLogCallback to include the request and response objects themselves rather than just include the data in the log message.

Motivation

Make it easier to parse networking logs.

Type Of Change

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

Testing Instructions

Try out the new BranchLogCallback with the request and response. This is a breaking change since it adds new fields to the BranchLogCallback

cc @BranchMetrics/saas-sdk-devs for visibility.

Copy link

@bredmond5 bredmond5 left a comment

Choose a reason for hiding this comment

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

couple questions but seems good


if (self.logCallback) {

if (self.advancedLogCallback) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nsingh-branch Since advancedLogCallback is always non-null, its initialized in init, else if condition will never be executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the default init since it shouldn't be necessary to have both. All of the tests appear to be passing now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did the new automation succeed as well?

@NidhiDixit09 NidhiDixit09 merged commit d1b6465 into master Dec 11, 2024
13 of 14 checks passed
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.

4 participants