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

feat(*): extend concerto compare to include field validators #481

Merged
merged 2 commits into from
Aug 9, 2022

Conversation

sstone1
Copy link
Contributor

@sstone1 sstone1 commented Aug 9, 2022

Extend concerto compare so that it can compare number and string validators attached to a field.

This will catch:

  • Number validator added to a field (incompatible change)
  • Lower bound of a number validator increased (incompatible change)
  • Lower bound of a number validator decreased (compatible change)
  • Upper bound of a number validator decreased (incompatible change)
  • Upper bound of a number validator increased (compatible change)
  • String validator added to a field (incompatible change)
  • String validator regex pattern changed (incompatible change)
  • String validator regex flags changed (incompatible change)

Note that for regexes, it may be possible to make a regex more lenient than it was before, but there is no way for us to programmatically assert that one regex is more lenient than another regex.

@sstone1 sstone1 requested review from dselman and mttrbrts August 9, 2022 11:40
@sstone1 sstone1 self-assigned this Aug 9, 2022
Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

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

A thing of beauty. 👍

@@ -43,3 +43,13 @@ export function getPropertyType(property: Property) {
throw new Error(`unknown property type "${property}"`);
}
}

export function getValidatorType(validator: Validator) {
if (validator instanceof NumberValidator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not applicable here - but we've been bitten by using instanceof for exported types, as instanceof also checks the module for the type.

@sstone1 sstone1 merged commit ee709ff into accordproject:master Aug 9, 2022
@sstone1 sstone1 deleted the simon-stone/concerto-compare branch August 9, 2022 11:59
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