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

Proposal: Improve compatibility with JSON in configuration #653

Merged
merged 1 commit into from
Mar 21, 2018

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Mar 21, 2018

The configuration format from the server is almost already a superset of JSON with exception of the following two cases:

  1. trailing commas from values at the top level are not allowed(although they are supported inside of maps)

  2. initial and final brackets at the top level section not supported

This change relaxes the two cases above to just skip those tokens so that the config syntax can also support valid JSON objects for configuring the server which might be helpful when generating it programmatically.

  • Link to issue, e.g. Resolves #NNN
  • Documentation added (if applicable)
  • Tests added
  • Branch rebased on top of current master (git pull --rebase origin master)
  • Changes squashed to a single commit (described here)
  • Build is green in Travis CI
  • You have certified that the contribution is your original work and that you license the work to the project under the Apache 2 license

Changes proposed in this pull request:

  • Skip initial and final brackets from config syntax
  • Ignore trailing commas at the top level

/cc @nats-io/core

The configuration format from the server is almost already a superset
of JSON with exception of two cases:

1) trailing commas from values at the top level (although they are
   supported inside of maps)

2) initial and final brackets on the top level section.

This change relaxes the two cases above to just skip those tokens so
that the config syntax can also support valid JSON objects for
configuring the server which might be helpful when generating it
programmatically.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 92.184% when pulling c45d2f5 on wallyqs:conf-json-compat into b2a9ed9 on nats-io:master.

@derekcollison
Copy link
Member

LGTM

@derekcollison derekcollison merged commit 5573037 into nats-io:master Mar 21, 2018
@wallyqs wallyqs deleted the conf-json-compat branch March 21, 2018 18:28
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