Skip to content
This repository has been archived by the owner on Mar 11, 2023. It is now read-only.

Add ‘extended_tweet’ field to Tweet Class #206

Merged
merged 2 commits into from
Feb 18, 2018

Conversation

ryamaguchi0220
Copy link

This resolves #161

@codecov
Copy link

codecov bot commented Feb 18, 2018

Codecov Report

Merging #206 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #206   +/-   ##
=======================================
  Coverage   95.59%   95.59%           
=======================================
  Files          71       71           
  Lines        1136     1136           
  Branches       15       15           
=======================================
  Hits         1086     1086           
  Misses         50       50

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aef97f3...14a9b1a. Read the comment docs.

Copy link
Owner

@DanielaSfregola DanielaSfregola left a comment

Choose a reason for hiding this comment

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

Hi @ryamaguchi0220,
thanks for your contribution!

We are really close to merging this PR, we only need to add tests.

For example, you could add an extended tweet field to the json representing the data coming from twitter (see https://github.com/DanielaSfregola/twitter4s/blob/master/src/test/resources/twitter/rest/statuses/update.json) and make sure that the extended entity is produced in the corresponding fixture (see https://github.com/DanielaSfregola/twitter4s/blob/master/src/test/resources/fixtures/rest/statuses/update.json).

Adding to those json files should be enough to check that everything is working ;)

@ryamaguchi0220
Copy link
Author

Hi @DanielaSfregola,
Should I add more test?
The tweet structure seems to be used in various places.

Copy link
Owner

@DanielaSfregola DanielaSfregola left a comment

Choose a reason for hiding this comment

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

Hi @ryamaguchi0220,
thanks for adding the tests!

LGTM

We do have a ticket to write tests dedicated to the serialization/deserialization of our main entities, but for now, this will do -- since it is consistent with what we have done so.

@DanielaSfregola DanielaSfregola added this to the 5.5 milestone Feb 18, 2018
@DanielaSfregola
Copy link
Owner

For future reference: the ticket about writing dedicated tests to serialization is #109

@DanielaSfregola DanielaSfregola merged commit 7425ac4 into DanielaSfregola:master Feb 18, 2018
@DanielaSfregola
Copy link
Owner

Released in twitter4s 5.5

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.

A few changes to the tweet structure
2 participants