Skip to content

Conversation

@lidavidm
Copy link
Member

No description provided.

@lidavidm
Copy link
Member Author

This builds on top of ARROW-13737 to avoid merge conflicts. Leaving in draft until then.

@github-actions
Copy link

@lidavidm lidavidm force-pushed the arrow-13691 branch 2 times, most recently from b2d789f to 4b3f9dc Compare August 30, 2021 16:57
@lidavidm lidavidm marked this pull request as ready for review August 30, 2021 16:57
Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM. A couple minor comments.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't this exactly the same as AssertVarStdIsInvalid?

Copy link
Member

Choose a reason for hiding this comment

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

Nit: make this const?

Copy link
Member

Choose a reason for hiding this comment

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

Should this be called all_valid for clarity?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants