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

conform now guarantees order of configuration options #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

conform now guarantees order of configuration options #8

wants to merge 1 commit into from

Conversation

0xAX
Copy link
Member

@0xAX 0xAX commented Nov 20, 2015

No description provided.

@0xAX
Copy link
Member Author

0xAX commented Nov 20, 2015

@thz this patch provides functional that we talked about. Waiting for review from somebody.

It also provides test in the test/integration_test.exs ():

 test "for the complex data types" do
....

and config (test/confs/complex_example.conf) which represents current behaviour.

@grundrauschen
Copy link

I tried this test and get, for a more complex configuration, a response with wrong order.
For a configuration with test.*.foo and test.*.bar I tried the following in the app.conf

test.egg3.foo = "string1"
test.egg3.bar = "string2"
test.egg0.foo = "string3"
test.egg0.bar = "string4"
test.egg1.foo = "string5"
test.egg1.bar = "string6"

and the result in mix conform.effective was

...
{:test, 
  [egg3: [foo: "string1", bar: "string2"],
  egg1: [foo: "string5", bar: "string6"],
  egg0: [foo: "string3", bar: "string4]]},
...

So the order is not preserved in the resulting list.

@0xAX
Copy link
Member Author

0xAX commented Dec 4, 2015

Hello

I've update the PR.

cc: @thz @grundrauschen

@grundrauschen
Copy link

@0xAX: as far as I can tell from a few tests, the branch works now as intended.

@surik
Copy link
Member

surik commented Dec 8, 2015

The diff has a lot of unrelated changes with code style. Please clean it up.

@surik
Copy link
Member

surik commented Dec 8, 2015

Does somebody know usecase of this PR?
Is it really important to have this

...
{:test, 
  [egg3: [foo: "string1", bar: "string2"],
   egg0: [foo: "string3", bar: "string4"],
   egg1: [foo: "string5", bar: "string6]]},
...

instead of

{:test, 
  [egg3: [foo: "string1", bar: "string2"],
   egg1: [foo: "string5", bar: "string6"],
   egg0: [foo: "string3", bar: "string4]]},
...

in sys.config for

test.egg3.foo = "string1"
test.egg3.bar = "string2"
test.egg0.foo = "string3"
test.egg0.bar = "string4"
test.egg1.foo = "string5"
test.egg1.bar = "string6"

?

@surik
Copy link
Member

surik commented Dec 8, 2015

OK. Looks it is useful for some routing-rules with matching

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

Successfully merging this pull request may close these issues.

3 participants