-
Notifications
You must be signed in to change notification settings - Fork 35
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
Allow non-integer numbers in bundle.json #248
Conversation
Revert the changes in cnabio#257 which updated the spec to disallow non-integer values. We misunderstood the canonical json spec and were using a library that didn't support numbers. I have an open PRs for cnab-go which tested out switching to a library that support numbers in canonical json and it works great. * cnabio/cnab-go#247 * cnabio/cnab-go#248 So now fields such as default, maximum, minimum, etc can use numbers again! Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
* Allow numbers again Revert the changes in #257 which updated the spec to disallow non-integer values. We misunderstood the canonical json spec and were using a library that didn't support numbers. I have an open PRs for cnab-go which tested out switching to a library that support numbers in canonical json and it works great. * cnabio/cnab-go#247 * cnabio/cnab-go#248 So now fields such as default, maximum, minimum, etc can use numbers again! Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com> * Include integer and number in primitives list Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
3640b02
to
2227a77
Compare
#247 was merged, I think this only needs a rebase now? |
@radu-matei the tests won't pass until the spec is tagged. It is comparing my changes against the current spec, and so it's failing. Once I get Butcher's 👍 on the new release, then I can kick the build on this and it should be green. |
Now that we have switched to a canonical json library that supports non-integer numbers, I've updated the bundle.json definition to use float for: maximum, mimimum and multipleof. The tests now use non-integer numbers to validate that a bundle using values like 5.5 can be marshaled now. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
/brig run |
Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@radu-matei @vdice Okay, the spec change has been merged. This is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Now that we have switched to a canonical json library that supports
non-integer numbers, I've updated the bundle.json definition to use
float for: maximum, mimimum and multipleof.
The tests now use non-integer numbers to validate that a bundle using
values like 5.5 can be marshaled now.
🛑 This relies on #247 and upstream changes to the schemas in cnab-spec. Once that is merged and the spec tagged with a patch release, I'll rebase.