Zastai / MetaBrainz.MusicBrainz

Native .NET implementation of libmusicbrainz
MIT License
37 stars 9 forks source link

Add cancellation suport to async methods #38

Closed supermihi closed 2 years ago

supermihi commented 2 years ago

Hi, first of all thank you for this very well written library! One thing I noticed is that none of the async methods seem to accept an (optional) CancellationToken, contrary to the usual .NET practice (and the underlying methods such as HttpClient.SendAsync do support cancellation). Is this an explicit design decision or did just nobody dare to add the parameters yet?

Zastai commented 2 years ago

It was no real design decision other than avoiding even more parameters and/or overloads. Plus, no one asked for it, probably because no one was using the library asynchronously.

As I mentioned in a comment on #37, I am now working on adding such tokens everywhere (usually as sn optional argument, which does break binary compatibility).

Zastai commented 2 years ago

Once #37 is merged, I will prepare a branch (and PR) for these changes.

supermihi commented 2 years ago

Oops sorry, I overlooked that comment. I agree that optional arguments are preferable, that also seems to be the standard in system libraries.