Skip to content

Endpoints Search.repositories and Search.users#30

Merged
ChristopherDavenport merged 6 commits intodavenverse:masterfrom
timo-schmid:endpoint-search-streaming
Dec 6, 2019
Merged

Endpoints Search.repositories and Search.users#30
ChristopherDavenport merged 6 commits intodavenverse:masterfrom
timo-schmid:endpoint-search-streaming

Conversation

@timo-schmid
Copy link
Copy Markdown
Contributor

@timo-schmid timo-schmid commented Oct 6, 2019 •
edited
Loading

Read first

Changes

  • Make Search.repositories endpoint streaming
  • Create Search.users
  • Better tests for Search.repositories
  • Add tests for Search.users
  • Create tools for testing:
    • Paginate (creates pagination headers)
    • JsonFiles (reads Json files from the resource folder)
    • PaginatedJsonFiles (combines the above two into an endpoint for testing)

A few words

  • When testing multi-page results, the Json-data got huge. I decided to move it into resource files
  • The tests depend on the fix in Fix pagination beyond page 2 #29 so they will fail until that is fixed first.
  • I tried to split it up into 2 separate PR's but, I made changes in one common file and couldn't easily separate it, so now there is 1. Sorry 😅
  • Some fields in User apparently are optional, so I changed them to be an Option.

@timo-schmid
Endpoints Search.repositories and Search.users
621bb4c 
* Make Search.repositories endpoint streaming
* Create Search.users
* Better tests for Search.repositories
* Add tests for Search.users
* Create tools for testing:
  * Paginate (creates pagination headers)
  * JsonFiles (reads Json files from the resource folder)
  * PaginatedJsonFiles (combines the above two into an endpoint for testing)
@timo-schmid timo-schmid mentioned this pull request Oct 6, 2019
Open
@timo-schmid timo-schmid mentioned this pull request Oct 6, 2019
Merged
@timo-schmid
Copy link
Copy Markdown
Contributor Author

timo-schmid commented Dec 5, 2019

I just realized this never passed the tests, so I fixed it.
Let me know if I can improve on anything!

Greetings

Timo

@ChristopherDavenport ChristopherDavenport merged commit 62e869a into davenverse:master Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristopherDavenport ChristopherDavenport ChristopherDavenport approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@timo-schmid @ChristopherDavenport