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

Allow non-integer numbers in bundle.json #248

Merged
merged 3 commits into from
Jun 2, 2021

Conversation

carolynvs
Copy link
Contributor

@carolynvs carolynvs commented May 6, 2021

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.

carolynvs added a commit to carolynvs/cnab-spec that referenced this pull request May 17, 2021
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>
carolynvs added a commit to cnabio/cnab-spec that referenced this pull request May 24, 2021
* 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>
@carolynvs carolynvs force-pushed the enable-numbers branch 2 times, most recently from 3640b02 to 2227a77 Compare May 27, 2021 17:44
@radu-matei
Copy link
Member

#247 was merged, I think this only needs a rebase now?

@carolynvs
Copy link
Contributor Author

@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>
@carolynvs
Copy link
Contributor Author

/brig run

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs marked this pull request as ready for review May 28, 2021 16:58
@carolynvs
Copy link
Contributor Author

@radu-matei @vdice Okay, the spec change has been merged. This is ready for review.

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

LGTM

@carolynvs carolynvs merged commit 0082cac into cnabio:main Jun 2, 2021
@carolynvs carolynvs deleted the enable-numbers branch June 2, 2021 14:22
@cnabio cnabio deleted a comment from marseko Dec 2, 2023
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.

None yet

3 participants