Skip to content

fix: Improve transport synchronization and consume response #260

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

Merged
merged 2 commits into from
Sep 14, 2020

Conversation

Swatinem
Copy link
Member

The synchronization fixes have been proposed in #172 originally

@Swatinem Swatinem requested a review from a team September 14, 2020 10:20
Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

@Swatinem I assume that this makes #259 redundant?

@Swatinem
Copy link
Member Author

@Swatinem I assume that this makes #259 redundant?

yes. And I updated your comments about the response log.

Comment on lines +255 to +256
Err(err) => { sentry_debug!("Failed to read sentry response: {}", err); },
Ok(text) => { sentry_debug!("Get response: `{}`", text); },
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Err(err) => { sentry_debug!("Failed to read sentry response: {}", err); },
Ok(text) => { sentry_debug!("Get response: `{}`", text); },
Err(err) => sentry_debug!("Failed to read sentry response: {}", err),
Ok(text) => sentry_debug!("Get response: `{}`", text),

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering how this does not fail rustfmt.

Copy link
Member Author

Choose a reason for hiding this comment

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

its inside the macro, so none of that stuff is rustfmt-ed, also, I needed to wrap this in a block because of the way the sentry_debug macro is built :-(

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks. At least the comment after the } should go usually.

@Swatinem Swatinem merged commit 6bfb54b into master Sep 14, 2020
@Swatinem Swatinem deleted the fix/transport-fixes branch September 14, 2020 14:40
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.

2 participants