Skip to content
This repository was archived by the owner on May 14, 2020. It is now read-only.

Fix wrong cc,bcc,to field names#19

Merged
bob983 merged 1 commit intomasterfrom
bugfix/wrong-cc-bcc-to-field-names
Feb 17, 2016
Merged

Fix wrong cc,bcc,to field names#19
bob983 merged 1 commit intomasterfrom
bugfix/wrong-cc-bcc-to-field-names

Conversation

@bob983
Copy link
Copy Markdown
Contributor

@bob983 bob983 commented Feb 16, 2016

Description

@teinemaa found out that cc, bcc and to fields are not serialised and parsed properly. He proposed a fix in #18.

This PR extends the fix by providing tests and keeping the Conversation and AbstractThread fields in place and only renaming the JSON fields via GSON @SerializedName annotation.

@vrto
Copy link
Copy Markdown

vrto commented Feb 17, 2016

One general observation: Not sure how long does it take to execute these tests, but I'd perhaps consider merging the test to test all cc/bcc/to fields ... up to you!

@vrto
Copy link
Copy Markdown

vrto commented Feb 17, 2016

Otherwise 👍 :)

@bob983
Copy link
Copy Markdown
Contributor Author

bob983 commented Feb 17, 2016

It takes 6 seconds to run them all. If possible I prefer small tests with one assert - it actually was done as one big test but I decided to split it into several small tests.

@vrto
Copy link
Copy Markdown

vrto commented Feb 17, 2016

Maybe it doesn't sound like an issue now, but it can get out of control easily. It happened to us on Core-API - at certain point we grouped similar assertions and improved exec times by couple of minutes :)

@bob983
Copy link
Copy Markdown
Contributor Author

bob983 commented Feb 17, 2016

I understand your point, but I see it as premature optimisation now - the tests only program WireMock, that is already started and call java code (that requests data from WireMock).

@vrto
Copy link
Copy Markdown

vrto commented Feb 17, 2016

Roger that.

@bob983 bob983 force-pushed the bugfix/wrong-cc-bcc-to-field-names branch from e40717e to a43a0c9 Compare February 17, 2016 15:22
@bob983 bob983 force-pushed the bugfix/wrong-cc-bcc-to-field-names branch from a43a0c9 to 37ed2eb Compare February 17, 2016 15:24
bob983 added a commit that referenced this pull request Feb 17, 2016
@bob983 bob983 merged commit cc443d8 into master Feb 17, 2016
@bob983 bob983 deleted the bugfix/wrong-cc-bcc-to-field-names branch February 17, 2016 15:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants