47degrees/github4s

Add support for create / update / delete file

kalexmills opened this issue · 4 comments

kalexmills commented

Would like feedback from maintainers on a preferred type for the file contents on update and delete. I coded it as a String, since that's the literal JSON value and passed responsibility for base64 encoding back to the caller (which probably ought to be changed).

Should we just accept base64 encoding responsibility and change the input type? I feel like an Array[Byte] or fs2.Stream[F,Byte] would be preferred.

EDIT: looks like if we did change it, it would be the first use of these types in the API.

BenFradet commented

I think it's fine for now to leave base64 encoding to the caller and transmit to the API what we receive without modifications.

kalexmills commented

@BenFradet just saw your comment.

I found that API annoying during testing (didn't want to pull in a base64 conversion lib myself), so I converted the API to use Array[Byte] in commit 02f1cb4.

Since the base64 requirement is due to GitHub's implementation, I can see an argument for the github4s handling it.

We can revert that commit if you prefer it the other way.

PR is #390

BenFradet commented

I think usually we try to be as transparent as possible and stick to what the underlying API says. However, if you find that your approach is motivated (seems like it'll be easier on the user in this case) then it's fine 👍 .

I'll review the PR later today.

  • 👍1