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

977 add digits rule #978

Merged
merged 3 commits into from
May 13, 2019

Conversation

john-tipper
Copy link
Contributor

Fix to #977.

Implementation notes:

Requires use of a digits field that is associated with a property, which in turn requires integerDigits and fractionalDigits to be set within that digits field.

For example (this assumes that the config option of isUseBigDecimals() returns true).

{
    "type" : "object",
    "properties" : {
        "decimal" : {
            "type" : "number",
            "digits" : {
                "integerDigits": 5,
                "fractionalDigits": 10
            }
        }
    }
}

Validation of integerDigits and fractionalDigits is currently restricted to only checking that they exist. In the event that either does not, DigitsRule will throw an NoSuchElementException.

john-tipper and others added 2 commits May 1, 2019 23:40
Fix joelittlejohn#975: Add support for root level enum titles and descriptions. (#…
Currently throws NoSuchElementException if either of integer and fractional digits are undefined.
@joelittlejohn
Copy link
Owner

I like this addition, but the "digits" object seems unnecessary to me (an extra layer of nesting with no particular benefit). Can you just support integerDigits and fractionDigits at the property level?

@john-tipper
Copy link
Contributor Author

john-tipper commented May 2, 2019 via email

@joelittlejohn
Copy link
Owner

Yes, sounds good. They must both be omitted or both be included 👍

Both integerDigits and fractionalDigits must be specified or else no
@digits annotation will be added.
- Amend tests to specify fractional values to end in non-zero numerals.
@john-tipper
Copy link
Contributor Author

Updated PR to require 2 properties in order to introduce a @Digits constraint, as per @joelittlejohn's suggestion.
Failure mode in the event only one property is specified is to do nothing.

@joelittlejohn joelittlejohn merged commit 9f60132 into joelittlejohn:master May 13, 2019
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.

2 participants