Skip to content
This repository has been archived by the owner on Jul 16, 2020. It is now read-only.

Add optional input values and test suite #31

Merged
merged 4 commits into from
Apr 26, 2018
Merged

Add optional input values and test suite #31

merged 4 commits into from
Apr 26, 2018

Conversation

kofalt
Copy link
Member

@kofalt kofalt commented Apr 26, 2018

Closes #25, #29. This feature was the breaking point for not having at least a basic validation suite, so that is added as well. Basic but functional. Comments welcome.


Is this a semantic or operational change? If so:

  • Increment the version in spec/readme.md
  • After merge, tag the version and update the release page

@lmperry
Copy link
Member

lmperry commented Apr 26, 2018

Thanks. LGTM

Copy link
Contributor

@ehlertjd ehlertjd left a comment

Choose a reason for hiding this comment

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

One minor doc correction

examples/test.py Outdated

def test_folders():
base = path.dirname(path.abspath(__file__))
print base
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️

spec/readme.md Outdated
@@ -150,6 +150,8 @@ Like the inputs, you can add JSON schema constraints as desired. Please specify

The example has named one config option, called "speed", which must be an integer between zero and three, and another called "coordinates", which must be a set of three floats.

In some cases, a configuration option may not have a safe default, and it only makes sense to sometimes omit it entirely. If that is the case, specify `"optional": "true"` on that config key.
Copy link
Contributor

Choose a reason for hiding this comment

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

"optional": true

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@ehlertjd ehlertjd left a comment

Choose a reason for hiding this comment

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

LGTM!

@kofalt kofalt merged commit 4cc366e into master Apr 26, 2018
@kofalt kofalt deleted the optional-config branch April 26, 2018 21:13
@kofalt kofalt mentioned this pull request Apr 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants