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 GitHub Topics, #380 #421

Merged
merged 1 commit into from Jun 19, 2017
Merged

Conversation

MichaelViveros
Copy link
Contributor

This fixes #380

Notes:

  • Replaced keywords with topics - I removed a project's keywords and replaced them with topics. I discussed this with @heathermiller and we thought that it would be better for Scaladex to support Github's topics instead of Scaladex's own keywords since a lot of projects are already using topics (way more than the number of projects using keywords). Merging topics and keywords would get messy from a coding perspective so I thought it was best to remove keywords. It should be possible for all the projects that had keywords to now use topics since I verified that all projects with keywords have a github repo.

  • Added live updates to project page - As discussed with @heathermiller , I added a method to update a project's repo info (including topics) and readme whenever that project's page is loaded. See the liveUpdate method in DataRepository.scala which gets called in projectPage. This may cause the page to take a second or two longer to load but I didn't notice any significant lag while testing locally. If this causes too much lag or if it causes scaladex to hit the GraphQL API rate limit, I can look into other options like utilizing Github's Webhooks.

  • GraphQL API rate limit error - I was getting an error when downloading topics through the GraphQL API about abusing the rate limit. I fixed it by adding a delay in processTopicsResponse in GithubDownload.scala if the error happens. I'm not sure why I was getting the error (I'm using the same code from the Rest API code that alternates between using 3 API keys, I verified the keys still had lots of calls remaining after getting the error). I reached out to github support a couple weeks ago but they're still looking into it.

  • Did not use Sangria (GraphQL Client for Scala) - Sangria is more useful if you have your own GraphQL API you need to implement containing your data (Ex. If you were github and you were making a GraphQL API). It makes it easy to define your schema, to define how to process queries on your schema (not as useful if you’re using someone else’s GraphQL API like how Scaladex is using Github's GraphQL API, lots of overhead). Queries with sangria are just strings so you don’t get anything extra by using Sangria for making queries. You can make Sangria objects for the result of a query, but I was able to just save the results to a json file and then parse it later using case classes (the same way Scaladex downloads/parses repo info and readme files using Github's REST API).

@MasseGuillaume
Copy link
Contributor

Awesome work. The only issue I saw was a null somewhere, prefer Option[T].

@MasseGuillaume MasseGuillaume merged commit a771d7a into scalacenter:master Jun 19, 2017
@MichaelViveros MichaelViveros deleted the topics branch June 21, 2017 18:49
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.

Merge GitHub topics with Scaladex keywords
2 participants