feat: add github service #377
Conversation
| search_type: 'code', | ||
| search_query: 'org:testOrg path:/ filename:package.json in:path' | ||
| }; | ||
|
|
||
| const result = await service.getActiveReposForSearchTypeAndQuery(criteria); | ||
| const criteria2 = { | ||
| search_query: 'org:testOrg path:/ filename:package.json in:path' |
There was a problem hiding this comment.
Should one of these include a property search_type set to "" or some falsey value?
There was a problem hiding this comment.
We could, but it shouldn't be necessary. It's an optional parameter and when undefined, it hits the default case. In fact, if search_type is set to "" or a falsy value, the code will error due to https://github.com/NerdWalletOSS/shepherd/pull/377/files/5ad0285e1d7b7e12f1dc57cdf63ba23b82772902..982c25c27099f41d87a347f7a2345c81bf825aa6#diff-a9d5c72f5483cdb150974d030822fe95aeeba82f63bf72ae4b2c929fa7e50fb5R111. There's also a test that covers a bad value being passed: https://github.com/NerdWalletOSS/shepherd/pull/377/files/5ad0285e1d7b7e12f1dc57cdf63ba23b82772902..982c25c27099f41d87a347f7a2345c81bf825aa6#diff-8cde3a22acd42e1d42cdd48cb278b9902dce34a3f629dcb790841f2f8154f9b4R247.
|
PR looks good to me. Agree that this PR meets the objective of phase 1. |
|
@jugaltheshah, just need conflicts resolved before merging. |
|
Did one last round of local testing. Looks good. |
# [1.14.0](v1.13.1...v1.14.0) (2021-02-11) ### Features * add github service ([#377](#377)) ([7bbd54a](7bbd54a))
|
🎉 This PR is included in version 1.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Issue 361 batch 1:
This PR starts work towards #361 by adding a Github service. Idea here is to abstract the calls to Github behind something so that the underlying requests then can be changed out without doing too much work on the adapter. It will also allow us the flexibility to cleanly mix-and-match REST & GraphQL calls as needed.
Note
While all Github (more specifically, octokit) calls were moved to the new service, the abstraction is a bit leaky in that some adapter calls are still tied directly to Github responses. While not ideal, I think this is adequate for now as this will be changing shortly with phase 2 and beyond. Overall, I believe this PR meets its objective of making the next phase(s) easier.