Fix client stats on empty repos

[Kadaaran]

Fix #912

After pairing with @d12 we've been encountering some issues on Classroom because of the 204.

This is just a first shot as I think I need help about the tests ✋
Is it better to add more tests to check the return value for both 200 and 204 ?
Is the 0 value still ok ? It's based on @kytrinyx 's words:

I would prefer to provide a good zero value that the client code wouldn't have to check

Should it be something else ?

Thank you 🙏

[kytrinyx]

Sorry, I realize that I was using unfamiliar jargon when I said zero value.

For example, a great zero value when there's an array, is an empty array (rather than nil), because then the code will loop over the empty array, and nothing will happen.

In this case, what does the client code expect? What data would let the client code keep doing what it's doing, but essentially be a no-op?

[Kadaaran]

@kytrinyx No no no sooorrry it's me ! 🤦‍♀️ First it returned nil but I thought it was a bad idea and I added 0 without actually even thinking about it my bad.

As for us (Classroom) what we do is that we fetch the data and we use the sum method to add all commits from the repository if there is commits. We added a check, so if the answer is nil then we return 0.

I imagine it's not the case for everyone and that an empty array will be easier to deal with 👍

[kytrinyx]

I think that if the response is usually an array, then an empty array would be the cleanest response.

[Kadaaran]

@kytrinyx Do you want me to add new specs to it ?

[kytrinyx]

@Kadaaran I'm so sorry I didn't get back to you on this. 🌷

I've taken another look at this, and I can't help but think that the API should never send back a 204, since this is a GET request.

I'm going to chat with @tarebyte about fixing the API rather than changing the response here.

I'll leave this open for now, until we get clarity on the API side.

[Kadaaran]

@kytrinyx No worries 🌻

Sure ! Keep me updated if you have amy mews on this subject.

[d12]

Hey @kytrinyx , is there any movement on this issue?

If not, do you know roughly what timeline we're looking at re: fixing this on the GitHub side? It's breaking Classroom, so if this is going to take time, it'd be great to have a 👍 for merging an octokit hotfix in the meantime. I can take on finishing this PR.

[tarebyte]

@d12 there isn't really any movement on this right now, feel free to take on the PR 😄