Simplified programmatic configuration for bootstrap providers #19#205
Simplified programmatic configuration for bootstrap providers #19#205gabikliot merged 4 commits intodotnet:masterfrom yevhen:programmatic_bootstrapper_config
Conversation
|
Hi @yevhen, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
|
I've also added utility extension method for IDictionary lookup. It's internal. I think there many places in code where it could be useful. Finding smth in dictionary is so common :) |
|
The build has failed on Jenkins and it has nothing to do with my changes. The test for observers has failed for some intermittent reasons (timeouts). |
There was a problem hiding this comment.
We definitely should not limit to only one impl. of a given Bootstrap Provider. One should be able to pass multiple providers of the exact same type. We currently allow that. The difference in this PR is in the line below:
var config = new ProviderConfiguration( properties ?? new Dictionary<string, string>(), fullName, fullName);
the last arg should be provider name, not fullName of the type.
It means the RegisterBootstrapProvider should accept a name argument.
There was a problem hiding this comment.
Ok. I'll add name parameter
|
Yevgeny, you can also look at some examples here: #201 |
|
I also tried to run the tests. They indeed fail. The problem is that in your PR you removed the line: Providers = new Dictionary<string, IProviderConfiguration>(); in the ProviderCategoryConfiguration.Load. It now puts a burden of making sure to init this property on the caller, and there were paths in the code that did not do that. I think it would be more robust to init it in the constructor. is it going back to C from C++? That's one of the pillars of OOP - encapsulation, right? Anyway, it is also very easy to run the tests, either in the VS or by the Test script. |
I didn't get it. Did you mean it's better to leave it as just an example and not try to add it to the framework? |
That's a bug. I'll fix it. |
No, the other way. I liked it. I said it's better then the example approach I used in the sample. |
As per your example (gabikliot@844e26b). And follows the usual idiom when working with non-immutable objects. |
Simplified programmatic configuration for bootstrap providers #19
No description provided.