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

Add optional 'body' field to HttpInterface. #334

Merged
merged 5 commits into from
Mar 13, 2017
Merged

Add optional 'body' field to HttpInterface. #334

merged 5 commits into from
Mar 13, 2017

Conversation

bretthoerner
Copy link

This is step 1 of #273, which just allows people to provide the body manually if they attach their own HttpInterface instance.

Not sure what size to trim the body to, or if we should allow overrides of that size?

}

/**
* Creates a an HTTP element for an {@link com.getsentry.raven.event.Event}.

Choose a reason for hiding this comment

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

"a an"

@@ -226,6 +243,9 @@ public boolean equals(Object o) {
if (serverName != null ? !serverName.equals(that.serverName) : that.serverName != null) {
return false;
}
if (body != null ? !body.equals(that.body) : that.body != null) {

Choose a reason for hiding this comment

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

Clever

* @param maxMessageLength maximum length of the string
* @return trimmed string
*/
public static String trimString(String string, int maxMessageLength) {

Choose a reason for hiding this comment

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

It might be worth considering ellipisizing this? I'm not sure what behavior is consistent among the other SDKs…?

@bretthoerner bretthoerner merged commit 10ac4a5 into master Mar 13, 2017
@bretthoerner bretthoerner deleted the http-body branch March 13, 2017 20:16
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