feat: add Create / Update / Delete requests

[kalexmills]

WIP For #388

Based on the APIs in the ticket.

Thanks to the maintainers for having such a well-factored project. This has been easy so far.

[kalexmills]

Only thing I'm waiting on is actually testing this against GitHub via Ammonite (which I will do soon), since it seems the integration tests are only used / needed for reads.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@7c7f8ed). Click here to learn what that means.
The diff coverage is 88.88%.

@@            Coverage Diff            @@
##             master     #390   +/-   ##
=========================================
  Coverage          ?   79.19%           
=========================================
  Files             ?       23           
  Lines             ?      548           
  Branches          ?        2           
=========================================
  Hits              ?      434           
  Misses            ?      114           
  Partials          ?        0

Impacted Files	Coverage Δ
...4s/src/main/scala/github4s/domain/Repository.scala	100% <ø> (ø)
...ub4s/src/main/scala/github4s/http/HttpClient.scala	13.79% <0%> (ø)
github4s/src/main/scala/github4s/Decoders.scala	84.21% <100%> (ø)
...ithub4s/interpreters/RepositoriesInterpreter.scala	95.23% <100%> (ø)
github4s/src/main/scala/github4s/Encoders.scala	96.42% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c7f8ed...ef42376. Read the comment docs.

[kalexmills]

Tested this and it seems to be working, so I'm marking as ready for review.

I did find that requiring the client to convert to base64 is annoying, so the API now takes an array of bytes and handles the base64 conversion for you.

Only issue with this is over the Stream[F, Byte] idea is it does require keeping the entire file in memory, but that's GitHub's API. I don't see how we could do a light memory footprint without streaming JSON which would require deeper changes.

[kalexmills]

One more tweak to edit the docs is in-bound, fyi here for reviewers.

[BenFradet]

LGTM overall, thanks 👍

That PR and #391 go me thinking maybe we need a repo just for gh4s integration tests, wdyt @juanpedromoreno ? 🤔

[BenFradet]

is this dependency transitive?

[kalexmills]

It's used like this in the AuthInterpreter also. I'm not sure where it's from. Shall we pin it to the build?

[BenFradet]

yes might be a good idea 👍

[kalexmills]

There are issues with relying on the Commit object which uses some custom decoding that powers through multiple layers of JSON at once. After further testing, I've noticed that object is only used in listCommits.

Users could see a JsonParseException if they try to use it.

Anyway the Commit object in Repository.scala is inappropriate for modeling these endpoints, so I'll need to make some changes.

[BenFradet]

LGTM 👍 modulo the version change

[fedefernandez]

Code looks good. I'm wondering if we should have a single operation for creating and updating, like in the GitHub API. From a user's perspective, it could be confusing what to use when you don't know if the file exists or not. This is solved by the API specifying a single operation ("Creates a new file or updates an existing file in a repository.")

[kalexmills]

@fedefernandez I do not think 'create or update' can function as an upsert. The update requires that you include the sha as a parameter, so the user has to know whether the file exists or not, and has to have retrieved it once before.

This allows them to do optimistic locking to avoid lost updates, but it's a little misleading, imo.

[fedefernandez]

@kalexmills oh I see. Code looks good to me!

Please, update your branch with master

[kalexmills]

@fedefernandez Thanks! Done.

[juanpedromoreno]

Thanks @kalexmills