Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support of custom implementation of ICollection<T> #1063

Merged
merged 1 commit into from
Sep 27, 2020
Merged

Conversation

neuecc
Copy link
Member

@neuecc neuecc commented Sep 26, 2020

fix for #1062, #836

  • Add non generic types support - IEnumerable, ICollection
  • Fix to support non generics ICollection<>, IDictioanry<,> implemented custom type
  • Fix ReadMe, changes support impl ICollection to IList. (ICollection does not have Add so can not support)
  • Changing order, prioritize IDictionary<,> than ICollection<>, I don't think this will be a breaking change,until now priorities were determined by <T>, <TKey, TValue>, so the Dictionary was determined by the Dictionary.

@neuecc neuecc requested a review from AArnott September 26, 2020 17:27
@neuecc
Copy link
Member Author

neuecc commented Sep 26, 2020

CI failed?

@AArnott
Copy link
Collaborator

AArnott commented Sep 27, 2020

@neuecc Since this adds new functionality I think we should retarget the PR to v2.2 (which I will ship just as soon as we have no open PRs or blocking issues for it).
If you want to fix the bug for v2.1 first, we can split your commit into two and merge just the fix into v2.1, but I don't know that it matters much since 2.2 is around the corner.

@neuecc
Copy link
Member Author

neuecc commented Sep 27, 2020

Ok to target v2.2.

@AArnott AArnott added this to the v2.2 milestone Sep 27, 2020
…support non generics `ICollection<>`, `IDictioanry<,>` implemented custom type
@AArnott AArnott changed the base branch from master to v2.2 September 27, 2020 14:11
@AArnott AArnott merged commit dc9cc1a into v2.2 Sep 27, 2020
@AArnott AArnott deleted the fix1062 branch September 27, 2020 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants