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

DictionaryAdapter: Inconistency in NameValueCollection overload behind FEATURE_DICTIONARYADAPTER_XML #422

Closed
rzontar opened this issue Dec 17, 2018 · 5 comments
Milestone

Comments

@rzontar
Copy link
Contributor

rzontar commented Dec 17, 2018

The IDictionaryAdapterFactory defines 2 overloads of GetAdapter that take a NameValueCollection.

In the interface the methods are behind the directive FEATURE_DICTIONARYADAPTER_XML IDictionaryAdapterFactory.cs#L64.
But the implementation shields only overloads that XmlNode DictionaryAdapterFactory.cs#L87.

Since System.Collections.Specialized is already package reference, i think the 2 overloads could be exposed. Otherwise a new FEATURE_DICTIONARYADAPTER_NAMEVALUECOLLECTION could be introduced.

I'm willing to contribute a PR.

@jonorossi
Copy link
Member

Makes sense, if the implementation can use NameValueCollection in .NET Standard then the interface can too. A pull request would be good.

Are you using DictionaryAdapter in your own projects?

@jonorossi
Copy link
Member

Thanks

@jonorossi
Copy link
Member

Changelog updated

@rzontar
Copy link
Contributor Author

rzontar commented Dec 17, 2018

Glad that worked out so well.

Just to answer your question. Yes, we only use the DictionaryAdapter part of Castle in our project. We are moving to netstandard 2.0 and after migrating a library suddenly the overloads were missing. Apparently, the netstandard 1.5 implementation was picked up. We solved that by multi-targeting to net461.

I would be fantastic if there was netstandard 2.0 build, but i have read the discussion about it in another thread.

Best,
Rok

@jonorossi
Copy link
Member

We'll have a .NET Standard 2.x build, but as you read in that other thread it can't be 2.0, but 2.1 should be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants