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

Hackernews #59

Merged
merged 6 commits into from Aug 13, 2019
Merged

Hackernews #59

merged 6 commits into from Aug 13, 2019

Conversation

ginglis13
Copy link
Collaborator

@ginglis13 ginglis13 commented Aug 12, 2019

Description

Work in progress Hackernews additions. Includes index frame, story frame (subreddit equivalent), and thread frame functionality. I more or less followed the structure for Reddit functionality, with differences being mainly in that HN's API is not quite on the same level as Reddit's. The thread frame needs to be fleshed out, but there are complications per discussion in #12 . Feel free to clone this branch to check out what's working, and more importantly, what's not working.

We should also probably change the target branch of this PR before any merging. Don't wanna bork master.

Fixes #12

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style of this project
  • I have performed a self-review of my own code
  • My code is self documenting
  • My changes generate no new major crashes/bugs

@ginglis13 ginglis13 changed the title Hackernews [WIP] Hackernews Aug 12, 2019
@sambattalio
Copy link
Collaborator

Love to see it 🚀

@ginglis13 ginglis13 added this to the v2 milestone Aug 12, 2019
@ginglis13
Copy link
Collaborator Author

I think I have found a solution: A different API provided by Algolia does a much better job when returning info about a post. My approach will be to keep the existing code in place with the official HN API and to use this API for posts only. Here is an example response for this HN article. As you can see, both APIs use the article's unique ID as the method for requesting the article's data. For contrast, here is the [HN official API response](a story: https://hacker-news.firebaseio.com/v0/item/8863.json?print=pretty) for the same post.

This newly discovered API is by no means lightning fast, but it is fast enough and much more dev friendly. Will update in the next 24 hrs.

@wtheisen
Copy link
Owner

I tested these changes locally and they seem to work well so far! Just needs to be polished a little more so it's completely usable, awesome work!!

@ginglis13
Copy link
Collaborator Author

ginglis13 commented Aug 13, 2019

Update

Using different API for threadView as mentioned above. This API returns comment and post text as raw HTML, so I used a python lib called html2text to strip html elements from text. Updated requirements.txt accordingly. Everything should be working now, let me know of any issues! I can make a separate PR once this is complete to update readme and test cases.

@ginglis13 ginglis13 changed the title [WIP] Hackernews Hackernews Aug 13, 2019
@wtheisen wtheisen merged commit 7e2982f into wtheisen:master Aug 13, 2019
@wtheisen
Copy link
Owner

Fantastic work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site: Add hackernews functionality
3 participants