-
Notifications
You must be signed in to change notification settings - Fork 33
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 additional tweet fields to TweetUtils; partially address #194. #254
Conversation
Codecov Report
@@ Coverage Diff @@
## master #254 +/- ##
==========================================
+ Coverage 70.14% 70.52% +0.37%
==========================================
Files 41 41
Lines 1025 1038 +13
Branches 191 191
==========================================
+ Hits 719 732 +13
Misses 240 240
Partials 66 66
Continue to review full report at Codecov.
|
assert(tweet.profileLocation() == null) | ||
assert(tweet.profileName() == null) | ||
assert(tweet.profileUrl() == null) | ||
assert(tweet.profileTimezone() == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use ".isEmpty" instead of == null, so scalastyle does not burp. Technically the return should be String instead of Null anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the assertions. Although this is strictly a style issue, so not mandatory. (e.g. assert(tweet.profileLocation().isEmpty)
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's out of scope for this PR, and should be a new ticket. Create a new ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. That weird - these should have been detected and resolved in #249. I will add the ticket and try to find out why scalastyle did not yell at me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with new categories and all looks good – just one minor typo fix and this is ready to merge.
def text(): String = try { (tweet \ "text").extract[String] } catch { case e: Exception => ""} | ||
/** Get the full_text. */ | ||
/** Get the the tweet full_text. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double "the the" here
- Adds: - retweet_count - favorite_count - in_reply_to_status_id_str - in_reply_to_user_id_str - in_reply_to_screen_name - source - user.protected - user.profile_image_url - user.description - user.location - user.name - user.url - user.time_zone - Updates some doc comments - Updates tests
@ianmilligan1 done! I rebased locally, so it's all a single commit with the commit message I want. So, feel free to squash and merge when you're ready. |
GitHub issue(s): #194
What does this Pull Request do?
Adds the following addition fields to TweetUtils:
How should this be tested?
Same as outlined in #252, but test additional fields.
Tests should take care of it all. But, definitely curious to see how it goes.
Additional Notes:
Still need to add some addition fields outlined in #194. Entity fields will be interesting since they are all an array.
Interested parties
@ianmilligan1 @greebie @lintool