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

Exclude min max annotations #122

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

codalogic
Copy link
Contributor

I've updated the parser and parser tests to support @{exclude-min}, @{exclude-max} and @{default ???}.

NOTE WELL!...

I haven't updated the validation side of things. evaluate_value_rules.rb:53 and evaluate_value_rules.rb:94 look relevant, but I think you also do some rationalisation of specified annotations before the code gets to that stage (e.g. evaluate-rules.rb:223). Rather than wade in and risk causing more trouble than good, I decided to leave that to you.

@coveralls
Copy link

coveralls commented Mar 22, 2018

Coverage Status

Coverage increased (+0.9%) to 97.596% when pulling a26cd68 on codalogic:exclude-min-max-annotations into 2e55b0e on arineng:master.

@anewton1998
Copy link
Contributor

anewton1998 commented Mar 22, 2018

Are we not doing @{default $rule_name} to reference objects, arrays, etc....?

@codalogic
Copy link
Contributor Author

I was hoping to avoid having to support @{default $rule_name}.

Arguably, just to check that a JCR syntax is valid you might have a validate that an object referenced by $rule_name was valid against the rule referencing it. That seemed like a lot of work, for an even rarer case than defaulted primitive values.

Do you think we should support @{default $rule_name}?

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