Refit and Polly

[hansmbakker]

I see this library has its own request logic and serialization logic.

To make these library parts more maintainable, I would recommend using Refit, and adding Polly for dealing with the rate limiting part.

[martincostello]

I would recommend against introducing Refit as a dependency.

As useful as it is, I've found through use in internal frameworks that the source generation it performs to create the interface implementations can cause issues when different versions of Refit are present in the complete dependency graph of an application.

For example when application A uses version Y of Refit and depends on library B which uses version X of Refit, there can be issues at runtime where implementation details of Refit embedded into B compiled against version X when the application runs using version Y.

This is easy-ish to manage internally as we can mandate updates to keep things in sync, but I think it would cause more trouble than it's worth for a public package in NuGet.org.

[hansmbakker]

If asked today whether I use Refit myself today, I would explore alternatives first. Refit has had a period of being unmaintained without acknowledging it, where PRs and github issues were simply ignored and asking about it seemed frowned upon (see https://github.com/reactiveui/refit/discussions/1447). It seems to be revived now but personally I was surprised with how the project's status seemed a taboo topic.

However, I would still recommend using some form of code generation for a REST client instead of writing the request / retry / serialization logic myself.

[martincostello]

Sure, I don't disagree that maybe using code generation is maybe better long term (the JavaScript Octokit API does this), I just think using Refit specifically to achieve that might solve problems for maintainers at the expense of rough edges for consumers.

These days I would have thought that a self-contained Roslyn source generator in this project itself would maybe be a better/more idiomatic way to go about it as it doesn't add new runtime dependencies for users of Octokit.

[nickfloyd]

Hey y'all! Thank you for the footwork, thoughts, and clarity here. We still have not had a chance to take a closer look at Refit - so your insight here is 🥇.

I will say that my tendency is to keep things as simple as possible until they can't be. Meaning, I am generally less aggressive when it comes to introducing dependencies when the built in framework or language can meet the need. Not that there is not space for something like this but I'd rather there be a compelling case as to why we'd want to introduce a new dependency.

I removed the up for grabs and hacktoberfest labels because this really needs more discussion and concrete benefit evaluation.

[github-actions[bot]]

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

[hansmbakker]

Unstale

[andremantaswow]

Can the new solution support serializing all types of Octokit.Activity ? Issues like this https://github.com/octokit/octokit.net/issues/2337 have not been fixed yet