Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Support for json arrays + some rewrite #186

Merged
merged 1 commit into from
Apr 28, 2015
Merged

Conversation

victorhurdugaci
Copy link
Contributor

Fixes #115

  • Support for JSON arrays
  • Use the json.net parser instead of manually parsing the json
  • Simplified some test code

cc @ChengTian @lodejard @glennc @anurse

@ghost ghost added the cla-already-signed label Apr 23, 2015

namespace Microsoft.Framework.ConfigurationModel
{
public class ConfigurationPathComparer : IComparer<string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is PathComparer the right name here? Should it not be Key comparer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't just compare individual keys. It can compare full config paths like settings:address:ip:0. See the unit tests for it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think path is a concept used by JSON.NET and we don't have such a concept in our configuration system. settings:address:ip:0 is a key in our understanding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will rename to ConfigurationKeyComparer

@victorhurdugaci
Copy link
Contributor Author

Updated

@ChengTian
Copy link
Contributor

:shipit:

- Use the json.net parser instead of manually parsing the json
- Simplified some test code
@@ -39,5 +39,6 @@
[assembly: AssemblyVersion("1.0.0.0")]
[assembly: AssemblyFileVersion("1.0.0.0")]
[assembly: InternalsVisibleTo("Microsoft.Framework.ConfigurationModel.Test")]
[assembly: InternalsVisibleTo("Microsoft.Framework.ConfigurationModel.Json.Test")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IVT is to be used only for the unit test assembly of this assembly. Make the type(s) in question public and move to a .Internal namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the PR that fixes this: #188

@victorhurdugaci victorhurdugaci restored the victorhu/arrays branch April 29, 2015 18:24
@victorhurdugaci victorhurdugaci deleted the victorhu/arrays branch April 29, 2015 18:26
@weitzhandler
Copy link

How do I read an array configuration value that's stored in config.json file?

@Eilon
Copy link
Member

Eilon commented Jul 22, 2015

@weitzhandler check out the various ArrayTests unit test classes in this PR, such as https://github.com/aspnet/Configuration/pull/186/files#diff-34ac638f35f8010bf56b528104f853c8R13

@weitzhandler
Copy link

@Eilon
Where does the GetSubKeys method reside?

@Eilon
Copy link
Member

Eilon commented Jul 22, 2015

@weitzhandler ah, those got renamed. See aspnet/Announcements#33.

@weitzhandler
Copy link

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

Successfully merging this pull request may close these issues.

5 participants