Fix #484: Use IEnumerable<T> instead of IList<T>.

[rossmills99]

Fix #484 by using IEnumerable in place of IList.

I've ended up expanding from the issue description to also update all places we used IList in interfaces and models.

As far as I can tell, this should not be a breaking change because every IList<T> is already an IEnumerable<T>, but IEnumerable<T> allows for greater flexibility in choice of the actual collection type implementations used.

[Arash-Sabet]

Reviewed and observed the changes.

[Arash-Sabet]

@rossmills99 Is this release's Nuget package available?

[DiscoPYF]

From what I understand, this is a breaking change. Any change from IList<T> to IEnumerable<T> in response models is a breaking change. (e.g. see the tests updates, Count to Count()).

The following code won't compile after the changes:

IList<string> projections = response.Projections; // CS0266 error

var projections = response.Projections;

// Anyting using IList<T> methods will cause errors, e.g. CS0021.

❓ Is it on purpose?

It's mostly beneficial for us, the library developers (or for customer implementation of serialization), to use IEnumerable<T> in response models, as we can change the implementation without breaking existing code. But honestly, I don't think it's worth introducing a breaking change, though it's a fairly minor one.

❓ Should we revert the change for response model and schedule it in a major release?

[rossmills99]

Using IList seems to have been a strange choice which got repeated in several places - not sure why it was used in the first place. But yes you're right to point out this is a breaking change. Happy to see this change be reverted, or alternatively we just treat it as a breaking change and bump the version per semantic versioning? i.e. why do we need to "schedule" something, we can just bump the version with this change, if it makes sense.

[DiscoPYF]

Using IList seems to have been a strange choice which got repeated in several places - not sure why it was used in the first place.

Not sure. If I had to guess, it's because IList<T> has a fixed Count property and the serializer uses List<T>. I agree that IEnumerable<T> is generally better, and callers can use .ToList() if they want a list.

Happy to see this change be reverted, or alternatively we just treat it as a breaking change and bump the version per semantic versioning? i.e. why do we need to "schedule" something, we can just bump the version with this change, if it makes sense.

I find it best to group breaking changes together and separate them from smaller enhancements. If someone updates to use the enhancements, they probably don't want to deal with breaking changes.

But yes, we can just bump the version. Were you thinking of bumping from 2.0.2 to 2.1.0 or 3.0.0? To follow the semantic versioning rules, we must bump to 3.0.0 but it makes it seem like a bigger update than it is.

I am working on #481 that we could release together before Christmas.

[rossmills99]

Semantic versioning doesn't say anything about the size of the update, so bumping to 3.0 should be fine I think.

[DiscoPYF]

@rossmills99 Is this release's Nuget package available?

Hey @Arash-Sabet , I would like to clarify the questions above with @rossmills99 before releasing this enhancement. We can get it out once done 👍

Depending on the outcome, it will be in a patch release, 2.0.2 or minor release 2.1.0

[Arash-Sabet]

Hey @rossmills99 & @DiscoPYF

Do you happen to know when you're planning to release the patch or minor release as the enhancements in our side are pending for your changes.

[rossmills99]

Per discussion above we can release as 3.0.0 along with a couple of other recently added improvements. I can prepare a release to go out today.

[Arash-Sabet]

@rossmills99 thanks for the prompt action.

[rossmills99]

@rossmills99 thanks for the prompt action.

No problem, thanks for the prompt 😄

The 3.0.0 package is now published in Nuget, you can view release notes here: https://github.com/ArangoDB-Community/arangodb-net-standard/releases/tag/3.0.0