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.

Sign up for GitHub

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

Jump to bottom

Require two elements in covering elements #210

Merged
merged 2 commits into from
May 6, 2024
Merged

Require two elements in covering elements #210

merged 2 commits into from
May 6, 2024

Conversation

m-mohr
Copy link
Collaborator

@m-mohr m-mohr commented Apr 27, 2024

The covering schemas for xmin, xmax etc. allow 0-2 elements in the array, but I think according to the spec it should always be exactly two elements.

@m-mohr m-mohr changed the title Require two elements in covering Require two elements in covering elements Apr 27, 2024
jwass
jwass reviewed Apr 28, 2024
View reviewed changes
Copy link
Collaborator

@jwass jwass left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks. Maybe add an invalid example to make sure zero or 1 length arrays will fail.

@m-mohr
Copy link
Collaborator Author

m-mohr commented Apr 28, 2024

I've added tests.

@m-mohr m-mohr requested a review from jwass April 28, 2024 21:52
jwass
jwass approved these changes Apr 29, 2024
View reviewed changes
Copy link
Collaborator

@jwass jwass left a comment

Choose a reason for hiding this comment

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

Thanks!

@cholmes cholmes added this to the 1.1 milestone May 2, 2024
jorisvandenbossche
jorisvandenbossche approved these changes May 6, 2024
View reviewed changes
Copy link
Collaborator

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks!

@jorisvandenbossche jorisvandenbossche merged commit 0128483 into main May 6, 2024
2 checks passed
@jorisvandenbossche jorisvandenbossche deleted the m-mohr-patch-2 branch May 6, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cholmes cholmes cholmes approved these changes

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

@jwass jwass jwass approved these changes

@kylebarron kylebarron kylebarron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@m-mohr @cholmes @jorisvandenbossche @jwass @kylebarron